[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);

I don't think this is a particularly good abstraction, as the debugserver path 
is still left here, and the path is definitely os- and arch-specific. It would 
be better if the API returned the path to the debugserver-to-be-used instead.

Bonus points if you can also hide the `DEBUGSERVER_BASENAME` logic from 
GDBRemoteCommunication.cpp into the same API.


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [PATCH] D82723: Add lldbAPI library to be able to invoke SB API from CommandObjects

[Pavel Labath via Phabricator via lldb-commits]

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

I think this needs a lot more discussion. Before accepting any change like 
this, I'd like to get consensus on the end goal of this project and the rough 
steps to get there.

As it stands now, I don't believe this is a good change. It breaks down one of 
the few layering invariants we had (SB API uses everything else, but not vice 
versa) without any clear benefit. If the goal is to rewrite the commands on top 
of the SB API, then I think it should be possible to write command that _only_ 
use the SB API, and impossible (or at least very hard) to accidentally use the 
private api. This (and the follow-up patch demonstrates that) makes it very 
easy to mix SB and private objects within the same command, which doesn't clean 
up anything -- it just adds to the confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82723



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

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Another possibility would be to rename `[US]LongLong` to something like 
`Get[SZ]ExtInteger` (similar to `APInt::get[SZ]ExtValue`) to make it clear what 
it does, and ditch the other getter functions. I've mean meaning to move this 
class away from using host types at some point (it's not appropriate or useful 
for most of our uses), but I've thought it would be good to clean it up first. 
Now I think it may be better to do it the other way around


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [lldb] fe08ab5 - [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Xing GUO via lldb-commits]

Author: Xing GUO
Date: 2020-06-30T16:28:39+08:00
New Revision: fe08ab542bd6328a7906e38ae473cf655eb6a228

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

LOG: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

'InitialLength' is replaced with 'Format' (DWARF32 by default) and 'Length' in 
this patch.
Besides, test cases for DWARFv4 and DWARFv5, DWARF32 and DWARF64 is
added.

Reviewed By: jhenderson

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

Added: 


Modified: 
lldb/test/API/functionalities/source-map/a.yaml
lldb/unittests/Expression/DWARFExpressionTest.cpp
lldb/unittests/Symbol/Inputs/inlined-functions.yaml
lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
llvm/include/llvm/ObjectYAML/DWARFYAML.h
llvm/lib/ObjectYAML/DWARFEmitter.cpp
llvm/lib/ObjectYAML/DWARFVisitor.cpp
llvm/lib/ObjectYAML/DWARFYAML.cpp
llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
llvm/test/ObjectYAML/MachO/DWARF-debug_ranges.yaml
llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml
llvm/test/tools/llvm-gsymutil/ARM_AArch64/fat-macho-dwarf.yaml
llvm/test/tools/llvm-gsymutil/X86/mach-dwarf.yaml
llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all-with-dwarf.yaml
llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
llvm/tools/obj2yaml/dwarf2yaml.cpp
llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp

Removed: 




diff  --git a/lldb/test/API/functionalities/source-map/a.yaml 
b/lldb/test/API/functionalities/source-map/a.yaml
index 08fd9895d912..cf2fe7b75c4e 100644
--- a/lldb/test/API/functionalities/source-map/a.yaml
+++ b/lldb/test/API/functionalities/source-map/a.yaml
@@ -296,8 +296,7 @@ DWARF:
 - Attribute:   DW_AT_type
   Form:DW_FORM_ref4
   debug_info:  
-- Length:  
-TotalLength: 122
+- Length:  122
   Version: 4
   AbbrOffset:  0
   AddrSize:8

diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp 
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 572c585c6bfd..78c400c33d4e 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -94,8 +94,7 @@ TEST(DWARFExpression, DW_OP_convert) {
   "  - Attribute:   DW_AT_byte_size\n"
   "Form:DW_FORM_data1\n"
   "debug_info:\n"
-  "  - Length:\n"
-  "  TotalLength: 0\n"
+  "  - Length:  0\n"
   "Version: 4\n"
   "AbbrOffset:  0\n"
   "AddrSize:8\n"

diff  --git a/lldb/unittests/Symbol/Inputs/inlined-functions.yaml 
b/lldb/unittests/Symbol/Inputs/inlined-functions.yaml
index 14573da3498d..8ce92115a0b5 100644
--- a/lldb/unittests/Symbol/Inputs/inlined-functions.yaml
+++ b/lldb/unittests/Symbol/Inputs/inlined-functions.yaml
@@ -342,8 +342,7 @@ DWARF:
 - Attribute:   DW_AT_type
   Form:DW_FORM_ref4
   debug_info:  
-- Length:  
-TotalLength: 579
+- Length:  579
   Version: 4
   AbbrOffset:  0
   AddrSize:8

diff  --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index 44251e944df6..858aa61b19bb 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -56,8 +56,7 @@ TEST_F(DWARFASTParserClangTests,
   "  - Attribute:   DW_AT_byte_size\n"
   "Form:DW_FORM_data1\n"
   "debug_info:\n"
-  "  - Length:\n"
-  "  TotalLength: 0\n"
+  "  - Length:  0\n"
   "Version: 4\n"
   "AbbrOffset:  0\n"
   "AddrSize:8\n"

diff  --git a/llvm/include/llvm/ObjectYAML/DWARFYAML.h 
b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
index 73965dc6d570..a492972082ee 100644
--- a/llvm/include/llvm/ObjectYAML/DWARFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
@@ -117,7 +117,8 @@ struct Entry {
 };
 
 struct Unit {
-  InitialLength Length;
+  dwarf::DwarfFormat Format;
+  uint64_t Length;
   uint16_t Version;
   llvm::dwarf::UnitType Type; // Added in DWARF 5
   yaml::Hex64 AbbrOffset;

diff  --git a/llvm/lib/ObjectYAML/DWARFEmitter.cpp 
b/llvm/lib/ObjectYAML/DWARFEmitter.cpp
index 7f8de2c2f30b..67509ba52cf4 100644
--- a/llvm/lib/O

[Lldb-commits] [PATCH] D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Xing GUO via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe08ab542bd6: [DWARFYAML][debug_info] Replace 
'InitialLength' with 'Format' and 'Length'. 
(authored by Higuoxing).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82622

Files:
  lldb/test/API/functionalities/source-map/a.yaml
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/Symbol/Inputs/inlined-functions.yaml
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  llvm/include/llvm/ObjectYAML/DWARFYAML.h
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/ObjectYAML/DWARFVisitor.cpp
  llvm/lib/ObjectYAML/DWARFYAML.cpp
  llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
  llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
  llvm/test/ObjectYAML/MachO/DWARF-debug_ranges.yaml
  llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
  llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
  llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml
  llvm/test/tools/llvm-gsymutil/ARM_AArch64/fat-macho-dwarf.yaml
  llvm/test/tools/llvm-gsymutil/X86/mach-dwarf.yaml
  llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all-with-dwarf.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
  llvm/tools/obj2yaml/dwarf2yaml.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
  llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp

Index: llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
===
--- llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
+++ llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
@@ -1437,8 +1437,7 @@
 - Attribute:   DW_AT_high_pc
   Form:DW_FORM_addr
   debug_info:
-- Length:
-TotalLength: 52
+- Length:  52
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -1517,8 +1516,7 @@
 - Attribute:   DW_AT_high_pc
   Form:DW_FORM_data4
   debug_info:
-- Length:
-TotalLength: 44
+- Length:  44
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -1617,8 +1615,7 @@
 - Attribute:   DW_AT_artificial
   Form:DW_FORM_flag_present
   debug_info:
-- Length:
-TotalLength: 68
+- Length:  68
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -1722,8 +1719,7 @@
 - Attribute:   DW_AT_high_pc
   Form:DW_FORM_data4
   debug_info:
-- Length:
-TotalLength: 78
+- Length:  78
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -1836,8 +1832,7 @@
 - Attribute:   DW_AT_call_line
   Form:DW_FORM_data4
   debug_info:
-- Length:
-TotalLength: 74
+- Length:  74
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -2092,8 +2087,7 @@
 - Attribute:   DW_AT_decl_line
   Form:DW_FORM_data1
   debug_info:
-- Length:
-TotalLength: 103
+- Length:  103
   Version: 4
   AbbrOffset:  0
   AddrSize:8
@@ -2342,8 +2336,7 @@
 - Attribute:   DW_AT_high_pc
   Form:DW_FORM_addr
   debug_info:
-- Length:
-TotalLength: 75
+- Length:  75
   Version: 4
   AbbrOffset:  0
   AddrSize:4
@@ -2485,8 +2478,7 @@
 - Attribute:   DW_AT_high_pc
   Form:DW_FORM_addr
   debug_info:
-- Length:
-TotalLength: 103
+- Length:  103
   Version: 4
   AbbrOffset:  0
   AddrSize:8
Index: llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
@@ -34,8 +34,7 @@
   - Attribute:   DW_AT_call_data_location
 Form:DW_FORM_sec_offset
 debug_info:
-  - Length:
-  TotalLength: 0
+  - Length:  0
 Version: 5
 UnitType:DW_UT_compile
 AbbrOffset:  0
Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -1368,8 +1368,7 @@
  "Children:DW_CHILDREN_yes\n"
  "Attributes:\n"
  "debug_info:\n"
- "  - Length:\n"
- "  TotalLength:  0\n"
+ "  - Length:  0\

[Lldb-commits] [lldb] 621f845 - [lldb] Modernize TestOperatorOverload.py asserts

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-06-30T11:38:42+02:00
New Revision: 621f84567920bf5a8d79789f01a622115ec8252d

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

LOG: [lldb] Modernize TestOperatorOverload.py asserts

Added: 


Modified: 
lldb/test/API/lang/cpp/operator-overload/TestOperatorOverload.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/operator-overload/TestOperatorOverload.py 
b/lldb/test/API/lang/cpp/operator-overload/TestOperatorOverload.py
index f541a6617e3a..c361a3916fb2 100644
--- a/lldb/test/API/lang/cpp/operator-overload/TestOperatorOverload.py
+++ b/lldb/test/API/lang/cpp/operator-overload/TestOperatorOverload.py
@@ -13,10 +13,6 @@ def test_overload(self):
 "break here", lldb.SBFileSpec("b.cpp"))
 frame = thread.GetSelectedFrame()
 value = frame.EvaluateExpression("x == nil")
-self.assertTrue(str(value.GetError())
-  .find("comparison between NULL and non-pointer ('Tinky' and NULL)")
-!= -1)
-self.assertTrue(str(value.GetError())
-  .find("invalid operands to binary expression ('Tinky' and")
-!= -1)
 self.assertFalse(value.GetError().Success())
+self.assertIn("comparison between NULL and non-pointer ('Tinky' and 
NULL)", str(value.GetError()))
+self.assertIn("invalid operands to binary expression ('Tinky' and", 
str(value.GetError()))



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

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Hey Pavel! Thanks for taking care of this. I started looking at it no long ago 
but I got sidetracked working on other stuff.
I don't see anything wrong with the patch, looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [PATCH] D82835: [lldb] Fix missing characters when autocompleting LLDB commands in REPL

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks!




Comment at: lldb/source/Expression/REPL.cpp:460
 sub_result.GetMatches(matches);
+// Prepend command prefix that was excluded in the completion request
+if (request.GetCursorIndex() == 0) {

Nit: Missing `.` at the end of the comment, but I'll fix that when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82835



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 274382.
omjavaid added a comment.

In this updated I have segregated RegisterInfoInterface and 
RegisterInfoAndSetInterface as two mutually exclusive interfaces. 
RegisterInfoPosix_arm64 is currently the only class making use of set interface 
but I am going to extend this once we reach a consensus. All occurrences of  
RegisterInfoPosix_arm64 have been updated accordingly and for the case of  
NativeRegisterContextLinux_arm64 RegisterInfoInterface shared pointer is casted 
into a RegisterInfoAndSetInterface shared pointer. Once we have this 
implemented for other architecture there wont be a need for having a shared 
pointer of RegisterInfoAndSetInterface in all variants of 
NativeRegisterContextLinux_*

@labath what are your thoughts on this ?


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

https://reviews.llvm.org/D80105

Files:
  lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
  
lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoAndSetInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -77,12 +77,13 @@
 ProcessElfCore *process = static_cast(GetProcess().get());
 ArchSpec arch = process->GetArchitecture();
 RegisterInfoInterface *reg_interface = nullptr;
+RegisterInfoAndSetInterface *regset_interface = nullptr;
 
 switch (arch.GetTriple().getOS()) {
 case llvm::Triple::FreeBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
+regset_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -111,7 +112,7 @@
 case llvm::Triple::NetBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
+regset_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
@@ -128,7 +129,7 @@
 reg_interface = new RegisterInfoPOSIX_arm(arch);
 break;
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
+regset_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::mipsel:
   case llvm::Triple::mips:
@@ -159,7 +160,7 @@
 case llvm::Triple::OpenBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
+regset_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -189,7 +190,7 @@
 switch (arch.GetMachine()) {
 case llvm::Triple::aarch64:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, regset_interface, m_gpregset_data, m_notes);
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm64(
   lldb_private::Thread &thread,
-  lldb_private::RegisterInfoInterface *register_info,
+  lldb_private::RegisterInfoAndSetInterface *register_info,
   const lldb_private::DataExtractor &gpregset,
   llvm::ArrayRef notes);
 
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 274390.
omjavaid added a comment.

In this updated I have removed overlapping parts of RegisterInfos_arm64.h and 
RegisterInfos_arm64_sve.h which in turn removes any possibility of duplicate 
definitions.

Both register infos define separate static register info arrays namely 
g_register_infos_arm64_le and g_register_infos_arm64_sve_le. Definition of FPR 
registers is also different and their invalidate/contains reg lists are also 
different. For the context of all native register context which do not yet 
support SVE g_register_infos_arm64_le will be used as they only include 
RegisterInfos_arm64.h.

@labath do you think there is still chance of any ODR violations?


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

https://reviews.llvm.org/D77047

Files:
  lldb/include/lldb/Target/RegisterContext.h
  lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/source/Utility/ARM64_DWARF_Registers.h
  lldb/source/Utility/ARM64_ehframe_Registers.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.core

Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
@@ -0,0 +1,24 @@
+// compile with -march=armv8-a+sve on compatible aarch64 compiler
+// linux-aarch64-sve.core was generated by: aarch64-linux-gnu-gcc-8
+// commandline: -march=armv8-a+sve -nostdlib -static -g linux-aarch64-sve.c
+static void bar(char *boom) {
+  char F = 'b';
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #7.5\n\t");
+  asm volatile("ptrue p1.s\n\t");
+  asm volatile("fcpy  z1.s, p1/m, #11.5\n\t");
+  asm volatile("ptrue p3.s\n\t");
+  asm volatile("fcpy  z3.s, p3/m, #15.5\n\t");
+
+  *boom = 47; // Frame bar
+}
+
+static void foo(char *boom, void (*boomer)(char *)) {
+  char F = 'f';
+  boomer(boom); // Frame foo
+}
+
+void _start(void) {
+  char F = '_';
+  foo(0, bar); // Frame _start
+}
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -269,7 +269,7 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
-@skipIfLLVMTargetMissing("AArch64")
+#@skipIfLLVMTargetMissing("AArch64")
 def test_aarch64_regs(self):
 # check 64 bit ARM core files
 target = self.dbg.CreateTarget(None)
@@ -323,6 +323,47 @@
 
 self.expect("register read --all")
 
+#@skipIf(triple='^mips')
+#@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_sve_regs(self):
+# check 64 bit ARM core files
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-sve.core")
+
+values = {}
+values["fp"] = "0xfc1ff4f0"
+values["lr"] = "0x00400170"
+values["sp"] = "0xfc1ff4d0"
+values["pc"] = "0x0040013c"
+values["v0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}"
+values["v1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}"
+values["v2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v3"] = "{0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41}"
+values["s0"] = "7.5"
+values["s1"] = "11.5"
+values["s2"] = "0"
+values["s3"] = "15.5"
+values["d0"] = "65536.0158538818"
+values["d1"] = "1572864.25476074"
+values["d2"] = "0"
+values["d3"] = "25165828.0917969"
+values["vg"] = "0x0004"
+values["z0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf

[Lldb-commits] [PATCH] D82853: [LLDB] Support custom expedited register set in gdb-remote

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: rengolin.
omjavaid added parent revisions: D79699: Add ptrace register access for AArch64 
SVE registers, D77047: AArch64 SVE register infos and core file support.

This patch adds capability to introduce a custom expedited register set in gdb 
remote. Currently we sent register set 0 as expedited register but for the case 
of AArch64 SVE we intend to send additional information about register set 
configuration stored in vg register. This will happen only when SVE mode is 
selected so we need a way to inform gdb-remote which register set to use when.


https://reviews.llvm.org/D82853

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -806,11 +806,17 @@
 
   // Grab the register context.
   NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p;
-  if (reg_ctx.GetRegisterSetCount() > 0 &&
-  ((reg_set_p = reg_ctx.GetRegisterSet(0)) != nullptr)) {
+  const RegisterSet *reg_set_p = reg_ctx.GetExpeditedRegisterSet();
+
+  if (reg_set_p == nullptr) {
+// Architecture does not specify an expedited register set.
+// Expedite all registers in the first register set (i.e. should be GPRs)
+// that are not contained in other registers.
+if (reg_ctx.GetRegisterSetCount() > 0)
+  reg_set_p = reg_ctx.GetRegisterSet(0);
+  }
+
+  if (reg_set_p != nullptr) {
 LLDB_LOGF(log,
   "GDBRemoteCommunicationServerLLGS::%s expediting registers "
   "from set '%s' (registers set count: %zu)",
Index: lldb/include/lldb/Host/common/NativeRegisterContext.h
===
--- lldb/include/lldb/Host/common/NativeRegisterContext.h
+++ lldb/include/lldb/Host/common/NativeRegisterContext.h
@@ -114,6 +114,8 @@
 
   virtual NativeThreadProtocol &GetThread() { return m_thread; }
 
+  virtual const RegisterSet *GetExpeditedRegisterSet() const { return nullptr; 
}
+
   const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
 uint32_t start_idx = 0);
 


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -806,11 +806,17 @@
 
   // Grab the register context.
   NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p;
-  if (reg_ctx.GetRegisterSetCount() > 0 &&
-  ((reg_set_p = reg_ctx.GetRegisterSet(0)) != nullptr)) {
+  const RegisterSet *reg_set_p = reg_ctx.GetExpeditedRegisterSet();
+
+  if (reg_set_p == nullptr) {
+// Architecture does not specify an expedited register set.
+// Expedite all registers in the first register set (i.e. should be GPRs)
+// that are not contained in other registers.
+if (reg_ctx.GetRegisterSetCount() > 0)
+  reg_set_p = reg_ctx.GetRegisterSet(0);
+  }
+
+  if (reg_set_p != nullptr) {
 LLDB_LOGF(log,
   "GDBRemoteCommunicationServerLLGS::%s expediting registers "
   "from set '%s' (registers set count: %zu)",
Index: lldb/include/lldb/Host/common/NativeRegisterContext.h
===
--- lldb/include/lldb/Host/common/NativeRegisterContext.h
+++ lldb/include/lldb/Host/common/NativeRegisterContext.h
@@ -114,6 +114,8 @@
 
   virtual NativeThreadProtocol &GetThread() { return m_thread; }
 
+  virtual const RegisterSet *GetExpeditedRegisterSet() const { return nullptr; }
+
   const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
 uint32_t start_idx = 0);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: rengolin.

In gdb-remote process we have register infos defind 
GDBRemoteDynamicRegisterInfo reference. In past register infos have remained 
constant througout the life time of a process.

This changes in AArch64 SVE variant where register infos will have per-thread 
configuration. SVE register will have per-thread size and can be updated while 
running. This patch aims to build up for that support by change 
GDBRemoteDynamicRegisterInfoSP from a reference defined in GDBRemoteProcess to 
a shared pointer deinfed per-thread.


https://reviews.llvm.org/D82857

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.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/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -14,6 +14,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/StructuredData.h"
 
+#include "GDBRemoteRegisterContext.h"
+
 class StringExtractor;
 
 namespace lldb_private {
@@ -101,6 +103,10 @@
   m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
+
+  void SetThreadRegisterInfo();
+
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data);
 
   bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -42,6 +42,9 @@
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
GetID());
+  // At this point we can clone reg_info for architectures supporting
+  // run-time update to register sizes and offsets..
+  SetThreadRegisterInfo();
 }
 
 ThreadGDBRemote::~ThreadGDBRemote() {
@@ -307,8 +310,8 @@
   !pSupported || gdb_process->m_use_g_packet_for_reading;
   bool write_all_registers_at_once = !pSupported;
   reg_ctx_sp = std::make_shared(
-  *this, concrete_frame_idx, gdb_process->m_register_info,
-  read_all_registers_at_once, write_all_registers_at_once);
+  *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once,
+  write_all_registers_at_once);
 }
   } else {
 reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
@@ -316,6 +319,23 @@
   return reg_ctx_sp;
 }
 
+void ThreadGDBRemote::SetThreadRegisterInfo() {
+  ProcessSP process_sp(GetProcess());
+  if (process_sp) {
+ProcessGDBRemote *gdb_process =
+static_cast(process_sp.get());
+
+if (!m_reg_info_sp) {
+  if (!gdb_process->m_register_info_sp->IsPerThreadRegInfo())
+m_reg_info_sp = gdb_process->m_register_info_sp;
+  else {
+m_reg_info_sp = std::make_shared();
+m_reg_info_sp->CloneFrom(gdb_process->m_register_info_sp);
+  }
+}
+  }
+}
+
 bool ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg,
   llvm::ArrayRef data) {
   GDBRemoteRegisterContext *gdb_reg_ctx =
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -251,7 +251,7 @@
  // the last stop
  // packet variable
   std::recursive_mutex m_last_stop_packet_mutex;
-  GDBRemoteDynamicRegisterInfo m_register_info;
+  GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
   HostThread m_async_thread;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -248,7 +248,7 @@
ListenerSP listener_sp)
 : Process(target_sp, listener_sp),
   m

[Lldb-commits] [PATCH] D82855: [LLDB] Send SVE vg register in custom expedited registerset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
omjavaid added parent revisions: D79699: Add ptrace register access for AArch64 
SVE registers, D77047: AArch64 SVE register infos and core file support.

This patch ovverides GetExpeditedRegisterSet for 
NativeRegisterContextLinux_arm64 to send vector granule register in expedited 
register set if SVE mode is selected.


https://reviews.llvm.org/D82855

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h


Index: lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
@@ -315,6 +315,7 @@
   k_last_sve_arm64 = sve_ffr_arm64,
 
   k_num_registers_arm64_sve,
+  k_num_expedited_registers_arm64_sve = gpr_cpsr_arm64 - gpr_x0_arm64 + 2,
   k_num_sve_registers_arm64 = k_last_sve_arm64 - k_first_sve_arm64 + 1
 
 };
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -58,6 +58,8 @@
 
   void ConfigureRegisterContext() override;
 
+  const RegisterSet *GetExpeditedRegisterSet() const override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -127,6 +127,23 @@
 enum { k_num_register_sets = 3 };
 }
 
+// ARM64 expedited expedited register set.
+static const uint32_t g_expedited_regnums_arm64_sve[] = {
+gpr_x0_arm64,  gpr_x1_arm64,   gpr_x2_arm64,  gpr_x3_arm64,
+gpr_x4_arm64,  gpr_x5_arm64,   gpr_x6_arm64,  gpr_x7_arm64,
+gpr_x8_arm64,  gpr_x9_arm64,   gpr_x10_arm64, gpr_x11_arm64,
+gpr_x12_arm64, gpr_x13_arm64,  gpr_x14_arm64, gpr_x15_arm64,
+gpr_x16_arm64, gpr_x17_arm64,  gpr_x18_arm64, gpr_x19_arm64,
+gpr_x20_arm64, gpr_x21_arm64,  gpr_x22_arm64, gpr_x23_arm64,
+gpr_x24_arm64, gpr_x25_arm64,  gpr_x26_arm64, gpr_x27_arm64,
+gpr_x28_arm64, gpr_fp_arm64,   gpr_lr_arm64,  gpr_sp_arm64,
+gpr_pc_arm64,  gpr_cpsr_arm64, sve_vg_arm64,  LLDB_INVALID_REGNUM};
+static_assert(
+((sizeof g_expedited_regnums_arm64_sve /
+  sizeof g_expedited_regnums_arm64_sve[0]) -
+ 1) == k_num_expedited_registers_arm64_sve,
+"g_expedited_regnums_arm64_sve has wrong number of register infos");
+
 // Register sets for ARM64.
 static const RegisterSet g_reg_sets_arm64[k_num_register_sets] = {
 {"General Purpose Registers", "gpr", k_num_gpr_registers_arm64,
@@ -135,6 +152,11 @@
  g_fpu_regnums_arm64},
 {"SVE Registers", "sve", k_num_sve_registers_arm64, g_sve_regnums_arm64}};
 
+// AArch64 SVE expedited registers set
+static const RegisterSet g_expedited_reg_sets_arm64_sve = {
+"General Purpose Registers", "expedited",
+k_num_expedited_registers_arm64_sve, g_expedited_regnums_arm64_sve};
+
 std::unique_ptr
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
@@ -1221,4 +1243,13 @@
 return m_sve_ptrace_payload.data();
   }
 
+  const RegisterSet *
+  NativeRegisterContextLinux_arm64::GetExpeditedRegisterSet() const {
+if (m_sve_state == SVE_STATE::SVE_STATE_FPSIMD ||
+m_sve_state == SVE_STATE::SVE_STATE_FULL)
+  return &g_expedited_reg_sets_arm64_sve;
+else
+  return &g_reg_sets_arm64[0];
+  }
+
 #endif // defined (__arm64__) || defined (__aarch64__)


Index: lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
@@ -315,6 +315,7 @@
   k_last_sve_arm64 = sve_ffr_arm64,
 
   k_num_registers_arm64_sve,
+  k_num_expedited_registers_arm64_sve = gpr_cpsr_arm64 - gpr_x0_arm64 + 2,
   k_num_sve_registers_arm64 = k_last_sve_arm64 - k_first_sve_arm64 + 1
 
 };
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@

[Lldb-commits] [lldb] 502773d - [lldb][NFC] Remove ImportInProgress lock in ClangASTSource

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-06-30T12:46:28+02:00
New Revision: 502773d743417a7896e184b4000bcf20fddb7ad9

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

LOG: [lldb][NFC] Remove ImportInProgress lock in ClangASTSource

Summary:

The ClangASTSource has a lock that globally disables all lookups into the
external AST source when we explicitly "guarded" copy a type. It's not used for
anything else, so importing declarations or importing types that are
dependencies of a declaration actually won't activate that lock. The lookups it
is supposed to prevent also don't actually happen in our test suite. The check
in `ClangExpressionDeclMap::FindExternalVisibleDecls` is never executed and the
check in the `ClangASTSource::FindExternalVisibleDeclsByName` is only ever
reached by the `Import-std-module` tests (which explicitly do a lookup into the
expression context on purpose).

This lock was added in 6abfabff6158076eccdf6fcac5a12894039de2c9 as a replacement
for a list of types we already looked up which appeared to be an optimisation
strategy. I assume back then this lock had a purpose but these days the
ASTImporter and LLDB seem to be smart enough to avoid whatever lookups this
tried to prevent.

I would say we remove it from LLDB. The main reason is that it blocks D81561
(which explicitly does a specific lookup to resolve placeholder types produced
by `-flimit-debug-info`) but it's semantics are also very confusing. The naming
implies it's a flag to indicate when we import something at the moment which is
practically never true as described above. Also the fact that it makes our
ExternalASTSource alternate between doing lookups into the debug info and
pretending it doesn't know any external decls could really break our lookup in
some weird way if Clang decides to cache a fake empty lookup result that was
generated while the lock was active.

Reviewers: labath, shafik, JDevlieghere, aprantl

Reviewed By: labath, JDevlieghere, aprantl

Subscribers: aprantl, abidh

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 4a81b1ae6a3b..6fe85a1298fc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -53,9 +53,9 @@ class ScopedLexicalDeclEraser {
 ClangASTSource::ClangASTSource(
 const lldb::TargetSP &target,
 const std::shared_ptr &importer)
-: m_import_in_progress(false), m_lookups_enabled(false), m_target(target),
-  m_ast_context(nullptr), m_ast_importer_sp(importer),
-  m_active_lexical_decls(), m_active_lookups() {
+: m_lookups_enabled(false), m_target(target), m_ast_context(nullptr),
+  m_ast_importer_sp(importer), m_active_lexical_decls(),
+  m_active_lookups() {
   assert(m_ast_importer_sp && "No ClangASTImporter passed to ClangASTSource?");
 }
 
@@ -103,11 +103,6 @@ bool ClangASTSource::FindExternalVisibleDeclsByName(
 return false;
   }
 
-  if (GetImportInProgress()) {
-SetNoExternalVisibleDeclsForName(decl_ctx, clang_decl_name);
-return false;
-  }
-
   std::string decl_name(clang_decl_name.getAsString());
 
   switch (clang_decl_name.getNameKind()) {
@@ -1744,13 +1739,9 @@ CompilerType ClangASTSource::GuardedCopyType(const 
CompilerType &src_type) {
   if (src_ast == nullptr)
 return CompilerType();
 
-  SetImportInProgress(true);
-
   QualType copied_qual_type = ClangUtil::GetQualType(
   m_ast_importer_sp->CopyType(*m_clang_ast_context, src_type));
 
-  SetImportInProgress(false);
-
   if (copied_qual_type.getAsOpaquePtr() &&
   copied_qual_type->getCanonicalTypeInternal().isNull())
 // this shouldn't happen, but we're hardening because the AST importer

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index 847a99abfff0..14761fbeb26b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -197,11 +197,6 @@ class ClangASTSource : public clang::ExternalASTSource,
 
   clang::Sema *getSema();
 
-  void SetImportInProgress(bool import_in_progress) {
-m_import_in_progress = import_in_progress;
-  }
-  bool GetImportInProgress() { return m_import_in_progress; }
-
   void SetLookupsEnabled(bool lookups_enabled) {
 m_lookups_enabled = lookups_enabled;
   }
@@ -376,7 +371,6

[Lldb-commits] [lldb] 80eb422 - [lldb] Tab completion for `frame select`

[Raphael Isemann via lldb-commits]

Author: Gongyu Deng
Date: 2020-06-30T13:13:38+02:00
New Revision: 80eb42281feb7834054be5f933f63d8c64af229d

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

LOG: [lldb] Tab completion for `frame select`

Summary: Provided the tab completion for command `frame select`.

Reviewers: teemperor, JDevlieghere

Reviewed By: teemperor

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectFrame.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index 5a61fc40e2e7..9ad4e3238359 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -289,6 +289,22 @@ class CommandObjectFrameSelect : public 
CommandObjectParsed {
 
   ~CommandObjectFrameSelect() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (!m_exe_ctx.HasProcessScope() || request.GetCursorIndex() != 0)
+  return;
+
+lldb::ThreadSP thread_sp = m_exe_ctx.GetThreadSP();
+const uint32_t frame_num = thread_sp->GetStackFrameCount();
+for (uint32_t i = 0; i < frame_num; ++i) {
+  lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i);
+  StreamString strm;
+  frame_sp->Dump(&strm, false, true);
+  request.TryCompleteCurrentArg(std::to_string(i), strm.GetString());
+}
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 4a548ad77083..b9ddba30a5e4 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -415,6 +415,13 @@ def test_completion_description_command_options(self):
 self.check_completion_with_desc("breakpoint set --Z", [
 ])
 
+def test_frame_select(self):
+self.build()
+self.main_source_spec = lldb.SBFileSpec("main.cpp")
+lldbutil.run_to_source_breakpoint(self, '// Break here', 
self.main_source_spec)
+
+self.complete_from_to('frame select ', ['0'])
+
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):
 self.build()



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

[Lldb-commits] [PATCH] D82759: [lldb/Test] Introduce "assertSuccess"

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.

LGTM, thanks!




Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:2537
+def assertSuccess(self, obj, msg=None):
+if not obj.Success():
+error = obj.GetCString()

JDevlieghere wrote:
> I'm not actually sure this works for custom types, but should we add an 
> `assert isinstance(obj, lldb.SBError)` to avoid misuse?
I actually also wondered if `obj` can be anything else in LLDB's SB API. I 
guess it's Pythonic to allow anything that fits that signature but some 
one-line documentaiton that this intended is for SBError would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82759



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

[Lldb-commits] [PATCH] D81177: [lldb] Tab completion for `frame select`

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG80eb42281feb: [lldb] Tab completion for `frame select` 
(authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81177

Files:
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -415,6 +415,13 @@
 self.check_completion_with_desc("breakpoint set --Z", [
 ])
 
+def test_frame_select(self):
+self.build()
+self.main_source_spec = lldb.SBFileSpec("main.cpp")
+lldbutil.run_to_source_breakpoint(self, '// Break here', 
self.main_source_spec)
+
+self.complete_from_to('frame select ', ['0'])
+
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):
 self.build()
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -289,6 +289,22 @@
 
   ~CommandObjectFrameSelect() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (!m_exe_ctx.HasProcessScope() || request.GetCursorIndex() != 0)
+  return;
+
+lldb::ThreadSP thread_sp = m_exe_ctx.GetThreadSP();
+const uint32_t frame_num = thread_sp->GetStackFrameCount();
+for (uint32_t i = 0; i < frame_num; ++i) {
+  lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i);
+  StreamString strm;
+  frame_sp->Dump(&strm, false, true);
+  request.TryCompleteCurrentArg(std::to_string(i), strm.GetString());
+}
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -415,6 +415,13 @@
 self.check_completion_with_desc("breakpoint set --Z", [
 ])
 
+def test_frame_select(self):
+self.build()
+self.main_source_spec = lldb.SBFileSpec("main.cpp")
+lldbutil.run_to_source_breakpoint(self, '// Break here', self.main_source_spec)
+
+self.complete_from_to('frame select ', ['0'])
+
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):
 self.build()
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -289,6 +289,22 @@
 
   ~CommandObjectFrameSelect() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (!m_exe_ctx.HasProcessScope() || request.GetCursorIndex() != 0)
+  return;
+
+lldb::ThreadSP thread_sp = m_exe_ctx.GetThreadSP();
+const uint32_t frame_num = thread_sp->GetStackFrameCount();
+for (uint32_t i = 0; i < frame_num; ++i) {
+  lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i);
+  StreamString strm;
+  frame_sp->Dump(&strm, false, true);
+  request.TryCompleteCurrentArg(std::to_string(i), strm.GetString());
+}
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81749: [lldb][NFC] Remove ImportInProgress lock in ClangASTSource

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG502773d74341: [lldb][NFC] Remove ImportInProgress lock in 
ClangASTSource (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81749

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -632,11 +632,6 @@
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  if (GetImportInProgress()) {
-LLDB_LOGV(log, "Ignoring a query during an import");
-return;
-  }
-
   if (log) {
 if (!context.m_decl_context)
   LLDB_LOG(log,
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -197,11 +197,6 @@
 
   clang::Sema *getSema();
 
-  void SetImportInProgress(bool import_in_progress) {
-m_import_in_progress = import_in_progress;
-  }
-  bool GetImportInProgress() { return m_import_in_progress; }
-
   void SetLookupsEnabled(bool lookups_enabled) {
 m_lookups_enabled = lookups_enabled;
   }
@@ -376,7 +371,6 @@
 
   friend struct NameSearchContext;
 
-  bool m_import_in_progress;
   bool m_lookups_enabled;
 
   /// The target to use in finding variables and types.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -53,9 +53,9 @@
 ClangASTSource::ClangASTSource(
 const lldb::TargetSP &target,
 const std::shared_ptr &importer)
-: m_import_in_progress(false), m_lookups_enabled(false), m_target(target),
-  m_ast_context(nullptr), m_ast_importer_sp(importer),
-  m_active_lexical_decls(), m_active_lookups() {
+: m_lookups_enabled(false), m_target(target), m_ast_context(nullptr),
+  m_ast_importer_sp(importer), m_active_lexical_decls(),
+  m_active_lookups() {
   assert(m_ast_importer_sp && "No ClangASTImporter passed to ClangASTSource?");
 }
 
@@ -103,11 +103,6 @@
 return false;
   }
 
-  if (GetImportInProgress()) {
-SetNoExternalVisibleDeclsForName(decl_ctx, clang_decl_name);
-return false;
-  }
-
   std::string decl_name(clang_decl_name.getAsString());
 
   switch (clang_decl_name.getNameKind()) {
@@ -1744,13 +1739,9 @@
   if (src_ast == nullptr)
 return CompilerType();
 
-  SetImportInProgress(true);
-
   QualType copied_qual_type = ClangUtil::GetQualType(
   m_ast_importer_sp->CopyType(*m_clang_ast_context, src_type));
 
-  SetImportInProgress(false);
-
   if (copied_qual_type.getAsOpaquePtr() &&
   copied_qual_type->getCanonicalTypeInternal().isNull())
 // this shouldn't happen, but we're hardening because the AST importer


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -632,11 +632,6 @@
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  if (GetImportInProgress()) {
-LLDB_LOGV(log, "Ignoring a query during an import");
-return;
-  }
-
   if (log) {
 if (!context.m_decl_context)
   LLDB_LOG(log,
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -197,11 +197,6 @@
 
   clang::Sema *getSema();
 
-  void SetImportInProgress(bool import_in_progress) {
-m_import_in_progress = import_in_progress;
-  }
-  bool GetImportInProgress() { return m_import_in_progress; }
-
   void SetLookupsEnabled(bool lookups_enabled) {
 m_lookups_enabled = lookups_enabled;
   }
@@ -376,7 +371,6 @@
 
   friend struct NameSearchContext;
 
-  bool m_import_in_progress;
   bool m_lookups_enabled;
 
   /// The target to use in finding variables and types.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/sourc

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: tschuett.
Herald added a reviewer: rengolin.
omjavaid added parent revisions: D82853: [LLDB] Support custom expedited 
register set in gdb-remote, D82855: [LLDB] Send SVE vg register in custom 
expedited registerset , D82857: [LLDB] Add per-thread register infos shared 
pointer in gdb-remote, D79699: Add ptrace register access for AArch64 SVE 
registers.

This patch builds on previously submitted SVE patches regarding expedited 
register set and per thread register infos. (D82853 
 D82855  and 
D82857 )

We need to resize SVE register based on value received in expedited list. Also 
we need to resize SVE registers when we write vg register using register write 
vg command. The resize will result in a updated offset for all of fpr and sve 
register set. This offset will be configured in native register context by 
RegisterInfoInterface and will be updated  by GDBRemoteProcess and 
GDBRemoteRegisterContext.

A follow up patch will provide a API test to verify this change.


https://reviews.llvm.org/D82863

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1762,6 +1762,26 @@
 gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
   }
 
+  // Code below is specific to AArch64 target in SVE state
+  // If expedited register set contains vector granule (vg) register
+  // then thread's register context reconfiguration is triggered by
+  // calling UpdateARM64SVERegistersInfos.
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+uint8_t arm64_sve_vg_dwarf_regnum = 46;
+GDBRemoteRegisterContext *reg_ctx_sp =
+static_cast(
+gdb_thread->GetRegisterContext().get());
+
+if (reg_ctx_sp) {
+  uint32_t vg_regnum = reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
+  eRegisterKindDWARF, arm64_sve_vg_dwarf_regnum);
+  if (expedited_register_map.count(vg_regnum)) {
+reg_ctx_sp->AArch64SVEReconfigure();
+  }
+}
+  }
+
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -40,6 +40,8 @@
 
   void HardcodeARMRegisters(bool from_scratch);
 
+  bool UpdateARM64SVERegistersInfos(uint64_t vg, uint32_t &end_reg_offset);
+
   void CloneFrom(GDBRemoteDynamicRegisterInfoSP process_reginfo);
 };
 
@@ -79,6 +81,8 @@
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
uint32_t num) override;
 
+  bool AArch64SVEReconfigure();
+
 protected:
   friend class ThreadGDBRemote;
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -347,6 +347,16 @@
   if (dst == nullptr)
 return false;
 
+  // Code below is specific to AArch64 target in SVE state
+  // If vector granule (vg) register is being written then thread's
+  // register context reconfiguration is triggered on success.
+  bool do_reconfigure_arm64_sve = false;
+  const ArchSpec &arch = process->GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+if (reg_info->kinds[eRegisterKindDWARF] == 46)
+  do_reconfigure_arm64_sve = true;
+  }
+
   if (data.CopyByteOrderedData(data_offset,// src offset
reg_info->byte_size,// src length
dst,// dst
@@ -366,6 +376,11 @@
 
 {
   SetAllRegisterValid(false);
+
+  if (do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(

[Lldb-commits] [lldb] c37d25f - [lldb] Tab completion for `frame recognizer delete`

[Raphael Isemann via lldb-commits]

Author: Gongyu Deng
Date: 2020-06-30T13:50:14+02:00
New Revision: c37d25f0d1e059b39fbbc1ccc9f52ddba42be6b9

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

LOG: [lldb] Tab completion for `frame recognizer delete`

Summary: Provided tab completion for command `frame recognizer delete`.

Reviewers: teemperor, JDevlieghere

Reviewed By: teemperor, JDevlieghere

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectFrame.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index 9ad4e3238359..5677b1612bee 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -936,6 +936,33 @@ class CommandObjectFrameRecognizerDelete : public 
CommandObjectParsed {
 
   ~CommandObjectFrameRecognizerDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (request.GetCursorIndex() != 0)
+  return;
+
+StackFrameRecognizerManager::ForEach(
+[&request](uint32_t rid, std::string rname, std::string module,
+   llvm::ArrayRef symbols,
+   bool regexp) {
+  StreamString strm;
+  if (rname.empty())
+rname = "(internal)";
+
+  strm << rname;
+  if (!module.empty())
+strm << ", module " << module;
+  if (!symbols.empty())
+for (auto &symbol : symbols)
+  strm << ", symbol " << symbol;
+  if (regexp)
+strm << " (regexp)";
+
+  request.TryCompleteCurrentArg(std::to_string(rid), strm.GetString());
+});
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (command.GetArgumentCount() == 0) {

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index b9ddba30a5e4..0cec206025d1 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -421,6 +421,10 @@ def test_frame_select(self):
 lldbutil.run_to_source_breakpoint(self, '// Break here', 
self.main_source_spec)
 
 self.complete_from_to('frame select ', ['0'])
+
+def test_frame_recognizer_delete(self):
+self.runCmd("frame recognizer add -l py_class -s module_name -n 
recognizer_name")
+self.check_completion_with_desc('frame recognizer delete ', [['0', 
'py_class, module module_name, symbol recognizer_name']])
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):



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

[Lldb-commits] [PATCH] D82865: [LLDB] Add GetByteOffset to SBValue interface for reading register offset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: rengolin.
omjavaid added a child revision: D82866: [LLDB] Test SVE dynamic resize with 
multiple threads.

This patch adds GetByteOffset to SBValue interface  for accessing register 
offset from register infos. This was needed to write tests to verify that 
AArch64 register sizes and offsets are updated when a thread updates vector 
granule register vg. A follow up test will make use of this API to test LLDB 
SVE register in multithreaded mode with dynamic update of size and offset.


https://reviews.llvm.org/D82865

Files:
  lldb/bindings/interface/SBValue.i
  lldb/include/lldb/API/SBValue.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp

Index: lldb/source/Core/ValueObjectSyntheticFilter.cpp
===
--- lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -123,6 +123,10 @@
 
 uint64_t ValueObjectSynthetic::GetByteSize() { return m_parent->GetByteSize(); }
 
+uint64_t ValueObjectSynthetic::GetByteOffset() {
+  return m_parent->GetByteOffset();
+}
+
 lldb::ValueType ValueObjectSynthetic::GetValueType() const {
   return m_parent->GetValueType();
 }
Index: lldb/source/Core/ValueObjectRegister.cpp
===
--- lldb/source/Core/ValueObjectRegister.cpp
+++ lldb/source/Core/ValueObjectRegister.cpp
@@ -231,6 +231,8 @@
 
 uint64_t ValueObjectRegister::GetByteSize() { return m_reg_info.byte_size; }
 
+uint64_t ValueObjectRegister::GetByteOffset() { return m_reg_info.byte_offset; }
+
 bool ValueObjectRegister::UpdateValue() {
   m_error.Clear();
   ExecutionContext exe_ctx(GetExecutionContextRef());
Index: lldb/source/API/SBValue.cpp
===
--- lldb/source/API/SBValue.cpp
+++ lldb/source/API/SBValue.cpp
@@ -339,6 +339,20 @@
   return result;
 }
 
+size_t SBValue::GetByteOffset() {
+  LLDB_RECORD_METHOD_NO_ARGS(size_t, SBValue, GetByteOffset);
+
+  size_t result = 0;
+
+  ValueLocker locker;
+  lldb::ValueObjectSP value_sp(GetSP(locker));
+  if (value_sp) {
+result = value_sp->GetByteOffset();
+  }
+
+  return result;
+}
+
 bool SBValue::IsInScope() {
   LLDB_RECORD_METHOD_NO_ARGS(bool, SBValue, IsInScope);
 
Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -38,6 +38,8 @@
 
   uint64_t GetByteSize() override;
 
+  uint64_t GetByteOffset() override;
+
   ConstString GetTypeName() override;
 
   ConstString GetQualifiedTypeName() override;
Index: lldb/include/lldb/Core/ValueObjectRegister.h
===
--- lldb/include/lldb/Core/ValueObjectRegister.h
+++ lldb/include/lldb/Core/ValueObjectRegister.h
@@ -88,6 +88,8 @@
 
   uint64_t GetByteSize() override;
 
+  uint64_t GetByteOffset() override;
+
   lldb::ValueType GetValueType() const override {
 return lldb::eValueTypeRegister;
   }
Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -46,6 +46,8 @@
 
   size_t GetByteSize();
 
+  size_t GetByteOffset();
+
   bool IsInScope();
 
   lldb::Format GetFormat();
Index: lldb/bindings/interface/SBValue.i
===
--- lldb/bindings/interface/SBValue.i
+++ lldb/bindings/interface/SBValue.i
@@ -89,6 +89,9 @@
 size_t
 GetByteSize ();
 
+size_t
+GetByteOffset ();
+
 bool
 IsInScope ();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82864: [lldb] Replace host-typed Scalar accessors with [SZ]ExtOrTruncInt

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, mib, teemperor.
Herald added a subscriber: emaste.
Herald added a reviewer: shafik.
Herald added a project: LLDB.

The Scalar class gives off the impression that it follows the C type
conversion rules, but this is not true for its host-typed accessor
methods. For example, Scalar((int)-1).ULongLong() returns 0x,
but (unsigned long long)(int)-1 is 0x. I.e., Scalar's
ULongLong always zero-extends, but C semantics require sign-extension.

This patch renames [SU]LongLong to [SZ]ExtOrTruncInt to better reflect
the conversion being done (name chosen after the underlying APInt
operation). Accessors for smaller types are removed completely as the
are not necessary -- normal C conversions from (u)int64_t will handle
that for us in most cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82864

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/include/lldb/Utility/Scalar.h
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectChild.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Utility/RegisterValue.cpp
  lldb/source/Utility/Scalar.cpp
  lldb/unittests/Utility/ScalarTest.cpp

Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -20,6 +20,9 @@
 using llvm::Failed;
 using llvm::Succeeded;
 
+template  static T (*max)() = std::numeric_limits::max;
+template  static T (*min)() = std::numeric_limits::min;
+
 template 
 bool checkInequality(T c1, T c2) {
   return (Scalar(c1) != Scalar(c2));
@@ -116,20 +119,24 @@
   ASSERT_EQ(0, memcmp(f, Storage, sizeof(f)));
 }
 
-TEST(ScalarTest, CastOperations) {
-  long long a = 0xf1f2f3f4f5f6f7f8LL;
-  Scalar a_scalar(a);
-  ASSERT_EQ((signed char)a, a_scalar.SChar());
-  ASSERT_EQ((unsigned char)a, a_scalar.UChar());
-  ASSERT_EQ((signed short)a, a_scalar.SShort());
-  ASSERT_EQ((unsigned short)a, a_scalar.UShort());
-  ASSERT_EQ((signed int)a, a_scalar.SInt());
-  ASSERT_EQ((unsigned int)a, a_scalar.UInt());
-  ASSERT_EQ((signed long)a, a_scalar.SLong());
-  ASSERT_EQ((unsigned long)a, a_scalar.ULong());
-  ASSERT_EQ((signed long long)a, a_scalar.SLongLong());
-  ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong());
+TEST(ScalarTest, ZExtOrTruncInt) {
+  EXPECT_EQ(uint64_t(max()), Scalar(max()).ZExtOrTruncInt());
+  EXPECT_EQ(uint64_t(max()),
+Scalar(max()).ZExtOrTruncInt());
+  EXPECT_EQ(uint64_t(max()), Scalar(max()).ZExtOrTruncInt());
+  EXPECT_EQ(uint64_t(max()),
+Scalar(max()).ZExtOrTruncInt());
+}
+
+TEST(ScalarTest, SExtOrTruncInt) {
+  EXPECT_EQ(int64_t(max()), Scalar(max()).SExtOrTruncInt());
+  EXPECT_EQ(int64_t(max()), Scalar(max()).SExtOrTruncInt());
+  EXPECT_EQ(int64_t(max()), Scalar(max()).SExtOrTruncInt());
+  EXPECT_EQ(int64_t(max()),
+Scalar(max()).SExtOrTruncInt());
+}
 
+TEST(ScalarTest, FloatCasts) {
   int a2 = 23;
   Scalar a2_scalar(a2);
   ASSERT_EQ((float)a2, a2_scalar.Float());
@@ -171,38 +178,38 @@
   EXPECT_EQ("12345", ScalarGetValue(12345));
   EXPECT_EQ("-12345", ScalarGetValue(-12345));
   EXPECT_EQ("12345", ScalarGetValue(12345));
-  EXPECT_EQ(std::to_string(std::numeric_limits::max()),
-ScalarGetValue(std::numeric_limits::max()));
+  EXPECT_EQ(std::to_string(max()),
+ScalarGetValue(max()));
 
   EXPECT_EQ("12345", ScalarGetValue(12345));
   EXPECT_EQ("-12345", ScalarGetValue(-12345));
   EXPECT_EQ("12345", ScalarGetValue(12345));
-  EXPECT_EQ(std::to_string(std::nu

[Lldb-commits] [PATCH] D82866: [LLDB] Test SVE dynamic resize with multiple threads

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
omjavaid added a parent revision: D82863: [LLDB] Add support to resize SVE 
registers at run-time.
omjavaid added a parent revision: D82865: [LLDB] Add GetByteOffset to SBValue 
interface for reading register offset.

This patch adds a new test case which depends on AArch64 SVE support and 
dynamic resize capability enabled. It created two seperate threads which have 
different values of sve registers and SVE vector granule at various points 
during execution.

We test that LLDB is doing the size and offset updates properly for all of the 
threads including the main thread and when we VG is updated using prctl call or 
by 'register write vg' command the appropriate changes are also update in 
register infos.


https://reviews.llvm.org/D82866

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/Makefile
  
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
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -0,0 +1,59 @@
+#include 
+#include 
+
+void *thread_func(void *x_void_ptr)
+{
+  void * return_ptr = NULL;
+  if (*((int *)x_void_ptr) == 0) {
+prctl(PR_SVE_SET_VL, 8 * 4);
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z30.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z31.s, p0/m, #5.\n\t");
+return_ptr = (void *)x_void_ptr;// Thread X breakpoint 1
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z0.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z1.s, p0/m, #5.\n\t");
+return return_ptr; // Thread X breakpoint 2
+  }
+
+  if (*((int *)x_void_ptr) == 1) {
+prctl(PR_SVE_SET_VL, 8 * 2);
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z30.s, p0/m, #5.\n\t");
+asm volatile("fcpy  z31.s, p0/m, #1.\n\t");
+return_ptr = x_void_ptr;// Thread Y breakpoint 1
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z1.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+return return_ptr;// Thread Y breakpoint 2
+  }
+  return return_ptr;
+}
+
+int main()
+{
+  prctl(PR_SVE_SET_VL, 8);
+
+  /* this variable is our reference to the second thread */
+  pthread_t x_thread, y_thread;
+
+  int x = 0, y = 1;
+
+  /* create a second thread which executes with argument x */
+  if(pthread_create(&x_thread, NULL, thread_func, &x)) // Break in main thread
+return 1;
+
+  /* create a second thread which executes with argument y */
+  if(pthread_create(&y_thread, NULL, thread_func, &y))
+return 1;
+
+  /* wait for the first thread to finish */
+  if(pthread_join(x_thread, NULL))
+return 2;
+
+  /* wait for the second thread to finish */
+  if(pthread_join(y_thread, NULL))
+return 2;
+
+  return 0;
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -0,0 +1,142 @@
+"""
+Test the AArch64 SVE registers dynamic resize with multiple threads.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size_and_offset(self, set, name, expected_size,
+   expected_offset):
+reg_value = set.GetChildMemberWithName(name)
+
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected_size,
+ 'Verify "%s" size == %i' % (name, expected_size))
+self.assertEqual(reg_value.GetByteOffset(), expected_offset,
+ 'Verify "%s" == %i' % (name, expected_offset))
+
+def check_sve_registers_config(self, set, vg_test_value):
+vg_reg_value = set.GetChildMemberWithName("vg").GetValueAsUnsigned()
+offset = set.GetChildMemberWithName("z0").GetByteOffset()
+
+z_reg_size = vg_reg_value * 8
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size_and_offset(
+set, 's%i' % (i), 4, offset)
+self.check_sve_register_size_and_offset(
+set, 'd

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for looking at this. Could you also tell me what you think of D82864 
? That's sort of a more safe/NFC-ish 
alternative to this patch, as it doesn't change behavior -- it  just makes the 
current behavior more obvious. (It's technically possible for the removal of 
the [US]Short and co. accessors to change behavior, but that would require a 
pretty complex set of conditions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [PATCH] D81241: [lldb] Tab completion for `frame recognizer delete`

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc37d25f0d1e0: [lldb] Tab completion for `frame recognizer 
delete` (authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81241

Files:
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -421,6 +421,10 @@
 lldbutil.run_to_source_breakpoint(self, '// Break here', 
self.main_source_spec)
 
 self.complete_from_to('frame select ', ['0'])
+
+def test_frame_recognizer_delete(self):
+self.runCmd("frame recognizer add -l py_class -s module_name -n 
recognizer_name")
+self.check_completion_with_desc('frame recognizer delete ', [['0', 
'py_class, module module_name, symbol recognizer_name']])
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -936,6 +936,33 @@
 
   ~CommandObjectFrameRecognizerDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (request.GetCursorIndex() != 0)
+  return;
+
+StackFrameRecognizerManager::ForEach(
+[&request](uint32_t rid, std::string rname, std::string module,
+   llvm::ArrayRef symbols,
+   bool regexp) {
+  StreamString strm;
+  if (rname.empty())
+rname = "(internal)";
+
+  strm << rname;
+  if (!module.empty())
+strm << ", module " << module;
+  if (!symbols.empty())
+for (auto &symbol : symbols)
+  strm << ", symbol " << symbol;
+  if (regexp)
+strm << " (regexp)";
+
+  request.TryCompleteCurrentArg(std::to_string(rid), strm.GetString());
+});
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (command.GetArgumentCount() == 0) {


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -421,6 +421,10 @@
 lldbutil.run_to_source_breakpoint(self, '// Break here', self.main_source_spec)
 
 self.complete_from_to('frame select ', ['0'])
+
+def test_frame_recognizer_delete(self):
+self.runCmd("frame recognizer add -l py_class -s module_name -n recognizer_name")
+self.check_completion_with_desc('frame recognizer delete ', [['0', 'py_class, module module_name, symbol recognizer_name']])
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_symbol_name(self):
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -936,6 +936,33 @@
 
   ~CommandObjectFrameRecognizerDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (request.GetCursorIndex() != 0)
+  return;
+
+StackFrameRecognizerManager::ForEach(
+[&request](uint32_t rid, std::string rname, std::string module,
+   llvm::ArrayRef symbols,
+   bool regexp) {
+  StreamString strm;
+  if (rname.empty())
+rname = "(internal)";
+
+  strm << rname;
+  if (!module.empty())
+strm << ", module " << module;
+  if (!symbols.empty())
+for (auto &symbol : symbols)
+  strm << ", symbol " << symbol;
+  if (regexp)
+strm << " (regexp)";
+
+  request.TryCompleteCurrentArg(std::to_string(rid), strm.GetString());
+});
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (command.GetArgumentCount() == 0) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82759: [lldb/Test] Introduce "assertSuccess"

[Pavel Labath via Phabricator via lldb-commits]

labath marked 3 inline comments as done.
labath added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:2537
+def assertSuccess(self, obj, msg=None):
+if not obj.Success():
+error = obj.GetCString()

teemperor wrote:
> JDevlieghere wrote:
> > I'm not actually sure this works for custom types, but should we add an 
> > `assert isinstance(obj, lldb.SBError)` to avoid misuse?
> I actually also wondered if `obj` can be anything else in LLDB's SB API. I 
> guess it's Pythonic to allow anything that fits that signature but some 
> one-line documentaiton that this intended is for SBError would be nice.
Yeah, duck-typing is very pythonic :P. At one point I actually had this 
special-case SBCommandReturnObject (It has `Succeeded` (:/) and `GetError` 
methods, but then I realized those are better handled by `runCmd` and friends.

I'll add some documentation about SBError.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82759



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

[Lldb-commits] [lldb] bc163f6 - [lldb] Fix TestModuleGetXcodeSDK test after DWARFYAML change

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-06-30T14:05:20+02:00
New Revision: bc163f63245e88f18de44f8a13a5c94e78cb18bd

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

LOG: [lldb] Fix TestModuleGetXcodeSDK test after DWARFYAML change

D82622 / fe08ab542bd6328a7906e38ae473cf655eb6a228 changes the YAML format
so this test was failing as the test yaml wasn't updated.

Added: 


Modified: 
lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index ac2c9fed9445..399eaf7aa648 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -47,8 +47,7 @@ TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   - Attribute:   DW_AT_APPLE_sdk
 Form:DW_FORM_strp
 debug_info:
-  - Length:
-  TotalLength: 8
+  - Length:  8
 Version: 2
 AbbrOffset:  0
 AddrSize:8



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

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

[Raphael Isemann via Phabricator via lldb-commits]

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

Beside some minor things this LGTM.




Comment at: lldb/source/Utility/Scalar.cpp:864
   case e_uint512:
-return static_cast(
-llvm::APIntOps::RoundAPIntToDouble(m_integer));

Seems unrelated to the patch? Also it would be inconsistent that this is 
removed here and not below. FWIW this is used to suppress `-Wdouble-promotion` 
warnings, so it does have a purpose.



Comment at: lldb/unittests/Utility/ScalarTest.cpp:97
+TEST(ScalarTest, Getters) {
+  CheckConversion((int)0x87654321);
+  CheckConversion((unsigned int)0x87654321u);

If you change this to `CheckConversion(0x87654321);` then that's one 
C-style cast less (which will make me very happy) and if someone accidentally 
makes this a `long` literal or so we get a compiler warning (instead of the 
compiler just silently truncating the thing).

Also I guess `short` and `char` is missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [PATCH] D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Ulrich Weigand via Phabricator via lldb-commits]

uweigand added a comment.

This commit causes test case failures on s390x (unfortunately hidden as the 
build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82622



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

[Lldb-commits] [PATCH] D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing added a comment.

In D82622#2122562 , @uweigand wrote:

> This commit causes test case failures on s390x (unfortunately hidden as the 
> build bot was already red due to an unrelated issue):
>  http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624
>
> http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml
>
> At first glance, seems like this could be endian-related?


Yes, if we don't specify the endianness in YAML, yaml2macho will emit binaries 
that have the same endianness as the host machine. I'll fix it. Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82622



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

[Lldb-commits] [lldb] 3567497 - [lldb/Test] Introduce "assertSuccess"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-06-30T15:41:03+02:00
New Revision: 35674976f09ec99e74d0d28b4a64b6bce360c128

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

LOG: [lldb/Test] Introduce "assertSuccess"

Summary:
A lot of our tests do 'self.assertTrue(error.Success()'. The problem
with that is that when this fails, it produces a completely useless
error message (False is not True) and the most important piece of
information -- the actual error message -- is completely hidden.

Sometimes we mitigate that by including the error message in the "msg"
argument, but this has two additional problems:
- as the msg argument is evaluated unconditionally, one needs to be
  careful to not trigger an exception when the operation was actually
  successful.
- it requires more typing, which means we often don't do it

assertSuccess solves these problems by taking the entire SBError object
as an argument. If the operation was unsuccessful, it can format a
reasonable error message itself. The function still accepts a "msg"
argument, which can include any additional context, but this context now
does not need to include the error message.

To demonstrate usage, I replace a number of existing assertTrue
assertions with the new function. As this process is not easily
automatable, I have just manually updated a representative sample. In
some cases, I did not update the code to use assertSuccess, but I went
for even higher-level assertion apis (runCmd, expect_expr), as these are
even shorter, and can produce even better failure messages.

Reviewers: teemperor, JDevlieghere

Subscribers: arphaman, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py

lldb/test/API/commands/expression/context-object-objc/TestContextObjectObjc.py
lldb/test/API/commands/expression/context-object/TestContextObject.py
lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
lldb/test/API/commands/expression/fixits/TestFixIts.py
lldb/test/API/commands/expression/options/TestExprOptions.py
lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py
lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
lldb/test/API/commands/expression/scoped_enums/TestScopedEnumType.py
lldb/test/API/commands/expression/timeout/TestCallWithTimeout.py
lldb/test/API/commands/expression/unwind_expression/TestUnwindExpression.py
lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
lldb/test/API/commands/register/register/register_command/TestRegisters.py

lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
lldb/test/API/python_api/hello_world/TestHelloWorld.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 481c23671228..35865d9e0d5c 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2481,9 +2481,7 @@ def expect_expr(
 else:
   eval_result = self.target().EvaluateExpression(expr, options)
 
-if not eval_result.GetError().Success():
-self.assertTrue(eval_result.GetError().Success(),
-"Unexpected failure with msg: " + 
eval_result.GetError().GetCString())
+self.assertSuccess(eval_result.GetError())
 
 if result_type:
 self.assertEqual(result_type, eval_result.GetDisplayTypeName())
@@ -2535,6 +2533,13 @@ def run_platform_command(self, cmd):
 err = platform.Run(shell_command)
 return (err, shell_command.GetStatus(), shell_command.GetOutput())
 
+"""Assert that an lldb.SBError is in the "success" state."""
+def assertSuccess(self, obj, msg=None):
+if not obj.Success():
+error = obj.GetCString()
+self.fail(self._formatMessage(msg,
+"'{}' is not success".format(error)))
+
 # =
 # Misc. helper methods for debugging test execution
 # =

diff  --git 
a/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py 
b/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
index f48cc5e6332b..a7e1a8c800cc 100644
--- a/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
+++ b/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
@@ -52,10 +52,7 @@ def call_function(self):
   'S

[Lldb-commits] [PATCH] D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing added a comment.

In D82622#2122562 , @uweigand wrote:

> This commit causes test case failures on s390x (unfortunately hidden as the 
> build bot was already red due to an unrelated issue):
>  http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624
>
> http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml
>
> At first glance, seems like this could be endian-related?


The failed tests are removed in 8032727a43ca678b0b923abaa04638f500a060d6 
. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82622



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

[Lldb-commits] [PATCH] D82759: [lldb/Test] Introduce "assertSuccess"

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rG35674976f09e: [lldb/Test] Introduce 
"assertSuccess" (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D82759?vs=274070&id=274459#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82759

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
  lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
  lldb/test/API/commands/expression/context-object-objc/TestContextObjectObjc.py
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
  lldb/test/API/commands/expression/fixits/TestFixIts.py
  lldb/test/API/commands/expression/options/TestExprOptions.py
  lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py
  lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
  lldb/test/API/commands/expression/scoped_enums/TestScopedEnumType.py
  lldb/test/API/commands/expression/timeout/TestCallWithTimeout.py
  lldb/test/API/commands/expression/unwind_expression/TestUnwindExpression.py
  lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
  lldb/test/API/python_api/hello_world/TestHelloWorld.py

Index: lldb/test/API/python_api/hello_world/TestHelloWorld.py
===
--- lldb/test/API/python_api/hello_world/TestHelloWorld.py
+++ lldb/test/API/python_api/hello_world/TestHelloWorld.py
@@ -142,7 +142,8 @@
 target.ConnectRemote(listener, None, None, error)
 
 process = target.AttachToProcessWithName(listener, name, False, error)
-self.assertTrue(error.Success() and process, PROCESS_IS_VALID)
+self.assertSuccess(error)
+self.assertTrue(process, PROCESS_IS_VALID)
 
 # Verify that after attach, our selected target indeed matches name.
 self.expect(
Index: lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
===
--- lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -57,47 +57,26 @@
 
 new_platform = lldb.SBPlatform(lldb.remote_platform.GetName())
 self.dbg.SetSelectedPlatform(new_platform)
-interpreter = self.dbg.GetCommandInterpreter()
 
 connect_url = "%s://%s:%s" % (protocol, hostname, str(hostport+1))
 
-command = "platform connect %s" % (connect_url)
-
-result = lldb.SBCommandReturnObject()
-
 # Test the default setting.
-interpreter.HandleCommand("settings show target.auto-install-main-executable", result)
-self.assertTrue(
-result.Succeeded() and
-"target.auto-install-main-executable (boolean) = true" in result.GetOutput(),
-"Default settings for target.auto-install-main-executable failed.: %s - %s" %
-(result.GetOutput(), result.GetError()))
+self.expect("settings show target.auto-install-main-executable",
+substrs=["target.auto-install-main-executable (boolean) = true"],
+msg="Default settings for target.auto-install-main-executable failed.")
 
 # Disable the auto install.
-interpreter.HandleCommand("settings set target.auto-install-main-executable false", result)
-interpreter.HandleCommand("settings show target.auto-install-main-executable", result)
-self.assertTrue(
-result.Succeeded() and
-"target.auto-install-main-executable (boolean) = false" in result.GetOutput(),
-"Default settings for target.auto-install-main-executable failed.: %s - %s" %
-(result.GetOutput(), result.GetError()))
+self.runCmd("settings set target.auto-install-main-executable false")
+self.expect("settings show target.auto-install-main-executable", 
+substrs=["target.auto-install-main-executable (boolean) = false"])
 
-interpreter.HandleCommand("platform select %s"%configuration.lldb_platform_name, result)
-interpreter.HandleCommand(command, result)
-
-self.assertTrue(
-result.Succeeded(),
-"platform process connect failed: %s - %s" %
-(result.GetOutput(),result.GetError()))
+self.runCmd("platform select %s"%configuration.lldb_platform_name)
+self.runCmd("platform connect %s" % (connect_url))
 
 # Create the target with the origin

Re: [Lldb-commits] [PATCH] D82155: [WIP][lldb/interpreter] Add ability to save lldb session to a file

[Pavel Labath via lldb-commits]

+ Shivam (Up until today I thought this was just theoretical musing but
now I realized that there is actually some work going towards this.)

On 25/06/2020 20:45, Jim Ingham wrote:
> 
> One of the things that is core functionality in the debugger is “when X
> happens, do Y”.  When I stop, do something, when I hit a breakpoint do
> something, when a new shared library is loaded (we don’t offer this
> affordance yet, but we should), etc…  Similarly the script interpreter
> side of lldb gets pretty involved in formatting data.  Both of these
> tasks require non-trivial managing.   I think it would be annoying for
> every debugger client to have to code that up themselves.  Some of that
> can be clearly expressed as generic debugger behavior (like running a
> condition on a breakpoint hit.)  But you also want a callout to more
> customizable behaviors.  At base, of course, these are just callbacks
> and don’t necessarily involve the script or command interpreters.

Yes, I was imagining these would be callbacks...

> But
> there’s a lot of bits of business particular to when the callbacks are
> running CLI or Script code, and it would be annoying to have every
> client have to reimplement these.
> 
> For instance, all CLI's will want to have to have a place to store the
> command callback text in the entities that are going to use them.  They
> would want that text to be part of the object’s “description” of itself.
>  And probably some other things that I’m forgetting.
And in that world, this information and the object "descriptions" would
probably not be parts of the objects themselves, but defined by the
particular CLI implementation (e.g., since the hypothetical gdb CLI
might want to format breakpoints differently than we normally do)...


> I think we could
> make these sockets sufficiently light-weight that having them in the
> core debugger wouldn’t have any practical downsides, and would make
> building on top of it easier.  That’s why I am inclined to include this
> part of the Command & Script interfaces in the CoreDebugger.
> 
> But I agree, the CoreDebugger should not include the implementation of a
> particular CommandInterpreter or a particular ScriptInterpreter, and
> should handle not having an actual CommandInterpreter present, as it can
> handle not having a ScriptInterpreter present.

... but I'm not really opposed to this design either (instead of a null
command interpreter, I might go for a no-op interpreter). I mainly want
to ensure that the roles and responsibilities of the different
components are well defined. The reason I'm going on about the gdb
emulator is not because I want to implement one (well.. maybe one day,
if I am extremely bored), but because it seems like a reasonable
criterion to decide which component does a piece of functionality go in.

> 
> I don’t see the layering problem, however.  In my view, there would be
> an abstract lldb_private::CommandInterpreter that receives text for
> execution returns results in a generic lldb_private::CommandResultObject
> (the current HandleCommand, HandleCommands).  It would provide an
> abstract “pretty print command text” call so that we could use it in the
> Description methods of lldb_private objects (for instance for
> breakpoints…)  It could also offer completion on types, filenames,
> variable names etc, since that’s a generic bit of functionality any CLI
> might want to use.  It might even have “HandleCompletion”, since that
> only knows that the command is text with a cursor…  But that’s about
> all.  This is clearly all within the lldb_private API’s.  We could
> probably also make “RunCommandInterpreter” sufficiently general that it
> might be worth adding that to the core functionality, and since this is
> a fairly complex bit of business because of I/O handlers and the like
> this seems worth trying.  But again, no SB API’s would be involved here.
>  Then there would be SP API’s that publish this functionality.  Again,
> this is just about storing and running the commands, so it doesn’t
> depend on any details of the interpreter.
> 
> Then for lldb, we would build a LLDBCommandInterpreter that uses the SB
> API’s to implement commands, and adds functionality like
> “DefineOptionGroup", “DefineCommand”, etc to set up commands in the
> command interpreter and would implement HandleCommand so that it could
> be run by the lldb_private::Debugger functionality.  But this would all
> be clearly on the outside of lldb_private.
> 

Right. I am not saying there necessarily _has_ to be a layering problem.
However, I do think that one has to design the thing carefully to ensure
he doesn't create one. I don't fully understand the design you're
proposing, but it does sound you've given it a lot of thought, and the
parts I do understand seem ok, and sound like they could be made to work.

However, the fact that D82723 starts by throwing everything into the
same bag does not sound reassuring to me. I'd like to understand how
those patches

[Lldb-commits] [lldb] e55a097 - [lldb/Scripts]

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-06-30T17:06:14+02:00
New Revision: e55a09793d689f42c17f55be861773b6a65751b2

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

LOG: [lldb/Scripts]

Fix analyze-project-deps.py. "lldb/Plugins" (home of Plugins.def) does
not depend on anything. Make sure this does not crash the script.

Added: 


Modified: 
lldb/scripts/analyze-project-deps.py

Removed: 




diff  --git a/lldb/scripts/analyze-project-deps.py 
b/lldb/scripts/analyze-project-deps.py
index c6e3263a2f26..a120260abfe2 100755
--- a/lldb/scripts/analyze-project-deps.py
+++ b/lldb/scripts/analyze-project-deps.py
@@ -109,7 +109,7 @@ def expand(path_queue, path_lengths, cycles, src_map):
 next_len = path_lengths.pop(0) + 1
 last_component = cur_path[-1]
 
-for item in src_map[last_component]:
+for item in src_map.get(last_component, []):
 if item.startswith("clang"):
 continue
 



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

[Lldb-commits] [PATCH] D82879: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: teemperor.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch improves the error reporting for SBBreakpoint::AddName by
changing its return type form a boolean to a SBError.

This way, if the breakpoint naming failed in the backend, the client
(i.e. Xcode), will be able to report the reason of that failure to the
user.

rdar://64765461

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82879

Files:
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/source/API/SBBreakpoint.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py

Index: lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -100,7 +100,7 @@
 
 # Add a name and make sure we match it:
 success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+self.assertTrue(success.Success(), "We couldn't add a legal name to a breakpoint.")
 
 matches = bkpt.MatchesName(bkpt_name)
 self.assertTrue(matches, "We didn't match the name we just set")
@@ -143,7 +143,7 @@
  "Cant Have Spaces"]
 for bad_name in bad_names:
 success = bkpt.AddName(bad_name)
-self.assertTrue(not success,"We allowed an illegal name: %s"%(bad_name))
+self.assertTrue(success.Fail(),"We allowed an illegal name: %s"%(bad_name))
 bp_name = lldb.SBBreakpointName(self.target, bad_name)
 self.assertFalse(bp_name.IsValid(), "We made a breakpoint name with an illegal name: %s"%(bad_name));
 
@@ -165,7 +165,7 @@
 
 # Add a name and make sure we match it:
 success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+self.assertTrue(success.Success(), "We couldn't add a legal name to a breakpoint.")
 
 bkpts = lldb.SBBreakpointList(self.target)
 self.target.FindBreakpointsByName(bkpt_name, bkpts)
@@ -244,7 +244,7 @@
 # Now add this name to a breakpoint, and make sure it gets configured properly
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 success = bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+self.assertTrue(success.Success(), "Couldn't add this name to the breakpoint")
 self.check_option_values(bkpt)
 
 # Now make a name from this breakpoint, and make sure the new name is properly configured:
@@ -318,7 +318,7 @@
 unprotected_id = unprotected_bkpt.GetID()
 
 success = protected_bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+self.assertTrue(success.Success(), "Couldn't add this name to the breakpoint")
 
 self.target.DisableAllBreakpoints()
 self.assertEqual(protected_bkpt.IsEnabled(), True, "Didnt' keep breakpoint from being disabled")
Index: lldb/source/API/SBBreakpoint.cpp
===
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -649,22 +649,23 @@
   return LLDB_RECORD_RESULT(sb_error);
 }
 
-bool SBBreakpoint::AddName(const char *new_name) {
-  LLDB_RECORD_METHOD(bool, SBBreakpoint, AddName, (const char *), new_name);
+SBError SBBreakpoint::AddName(const char *new_name) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpoint, AddName, (const char *), new_name);
 
   BreakpointSP bkpt_sp = GetSP();
 
+  SBError status;
   if (bkpt_sp) {
 std::lock_guard guard(
 bkpt_sp->GetTarget().GetAPIMutex());
-Status error; // Think I'm just going to swallow the error here, it's
-  // probably more annoying to have to provide it.
+Status error;
 bkpt_sp->GetTarget().AddNameToBreakpoint(bkpt_sp, new_name, error);
-if (error.Fail())
-  return false;
+status.SetError(error);
+  } else {
+status.SetErrorString("invalid breakpoint");
   }
 
-  return true;
+  return status;
 }
 
 void SBBreakpoint::RemoveName(const char *name_to_remove) {
@@ -1014,7 +1015,7 @@
(const char *, SBStructuredData &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, SetScriptCallbackBody,
(const char *));
-  LLDB_REGISTER_METHOD(bool, SBBreakpoint, AddName, (const char *));
+  LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, AddName, (const char *));
   LLDB_REGISTER_METHOD(void, SBBreakpoint, RemoveName, (const char *));
   LLDB_REGISTER_METHOD(bool, SBBreakpoint, MatchesName, (const char *));
   LLDB_REGISTER_

[Lldb-commits] [PATCH] D82864: [lldb] Replace host-typed Scalar accessors with [SZ]ExtOrTruncInt

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Looks good to me apart from one small detail.

I'll leave it open for others to comment on this, since there are lots of 
changes.




Comment at: lldb/source/Core/ValueObject.cpp:467
   bool ret;
-  ret = scalar_value.ULongLong(1) != 0;
+  ret = scalar_value != 0;
   error.Clear();

Any reason why this is not inlined in the definition ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82864



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

[Lldb-commits] [lldb] 69b2d9f - [lldb/Test] Skip recognizer tests when Python is disabled

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-30T08:59:44-07:00
New Revision: 69b2d9f42f6adbd0c06264dc42b4be5760689eac

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

LOG: [lldb/Test] Skip recognizer tests when Python is disabled

The `frame recognizer` command only exists when Python scripting is
enabled. Therefore the test should be made conditional on Python.
Without it, the test fails with "'frame recognizer' is not a known
command."

Added: 
lldb/test/Shell/Recognizer/lit.local.cfg

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/Recognizer/lit.local.cfg 
b/lldb/test/Shell/Recognizer/lit.local.cfg
new file mode 100644
index ..945a5615c421
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'python' not in config.available_features:
+  config.unsupported = True



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

[Lldb-commits] [PATCH] D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.

[Ulrich Weigand via Phabricator via lldb-commits]

uweigand added a comment.

In D82622#2122708 , @Higuoxing wrote:

> In D82622#2122562 , @uweigand wrote:
>
> > This commit causes test case failures on s390x (unfortunately hidden as the 
> > build bot was already red due to an unrelated issue):
> >  http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624
> >
> > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml
> >
> > At first glance, seems like this could be endian-related?
>
>
> The failed tests are removed in 8032727a43ca678b0b923abaa04638f500a060d6 
> . Thanks.


Thanks, the build bot is now green again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82622



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

[Lldb-commits] [PATCH] D82879: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

We can't break the SB API, it's unfortunate but this should be an overload 
instead...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82879



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

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM with Raphael's and clang-format's comments addressed :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [PATCH] D82378: [lldb/Unwind] Use eh_frame plan directly when it doesn't need to be augmented

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D82378#2113865 , @labath wrote:

> You're right that the instruction emulation not handling multiple epilogues 
> is a bug. It's probably not about _all_ epilogues, only some special ones -- 
> for example, the function where we ran into this was so simple it did not 
> have any traditional prologue instructions and I suspect this is what threw 
> the code off.
>
> I was planning to look into that separately.


I started looking into it now. The problem seems to be that the `pop %rcx` 
instruction is not treated as part of the epilogue -- the code only treats 
volatile register restores as a part of the epilogue. However, in this case, 
the `pop %rcx` is not there because the program needs to restore the register 
values -- it's there simply to undo the stack alignment that was done in the 
prologue to satisfy the ABI. This then causes us to incorrectly restore the 
"before-prologue" state when going past the first `ret`, and adjusting the 
stack twice:

  pushq %rax # align stack => CFA=rsp+16
  # some function call
  jcc 1f
  # some code
  pop %rcx # unalign stack => CFA=rsp+8
  retq # should reset CFA=rsp+16, but does +8 instead
  1: # more code, unwind info incorrect here
  pop %rcx # => CFA=rsp+0, should be +8
  retq

The simplest fix for that would be to treat _all_ pop instructions as the 
beginning of prologue (basically, move this 

 line out of the enclosing `if(non-volatile)` block), but I don't know whether 
that isn't too aggressive (it should be mostly ok, because current compilers 
prefer `movq ..., -N(%rsp)` instead of push/pop sequences, but still...). What 
do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82378



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

[Lldb-commits] [PATCH] D82616: Improve the detection of iOS/tvOS/watchOS simulator binaries in debugserver and lldb

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

Thanks.


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

https://reviews.llvm.org/D82616



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

davide marked an inline comment as done.
davide added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);

labath wrote:
> I don't think this is a particularly good abstraction, as the debugserver 
> path is still left here, and the path is definitely os- and arch-specific. It 
> would be better if the API returned the path to the debugserver-to-be-used 
> instead.
> 
> Bonus points if you can also hide the `DEBUGSERVER_BASENAME` logic from 
> GDBRemoteCommunication.cpp into the same API.
hmm, I wonder if this code should live in `GDBRemoteCommunication.cpp`


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [lldb] 0da0437 - Improve the detection of iOS/tvOS/watchOS simulator binaries in

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-06-30T11:22:03-07:00
New Revision: 0da0437b2afbd8ebef6b11f114cca33b118e7639

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

LOG: Improve the detection of iOS/tvOS/watchOS simulator binaries in
debugserver and lldb

This patch improves the heuristics for correctly identifying simulator binaries 
on Darwin and adds support for simulators running on Apple Silicon.

rdar://problem/64046344

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

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 6f3e8b637cf2..7b0d6f343c03 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -681,11 +681,16 @@ bool DynamicLoaderDarwin::AddModulesUsingImageInfos(
 loaded_module_list.AppendIfNeeded(image_module_sp);
   }
 
-  // macCatalyst support:
-  // Update the module's platform with the DYLD info.
+  // To support macCatalyst and legacy iOS simulator,
+  // update the module's platform with the DYLD info.
   ArchSpec dyld_spec = image_infos[idx].GetArchitecture();
-  if (dyld_spec.GetTriple().getOS() == llvm::Triple::IOS &&
-  dyld_spec.GetTriple().getEnvironment() == llvm::Triple::MacABI)
+  auto &dyld_triple = dyld_spec.GetTriple();
+  if ((dyld_triple.getEnvironment() == llvm::Triple::MacABI &&
+   dyld_triple.getOS() == llvm::Triple::IOS) ||
+  (dyld_triple.getEnvironment() == llvm::Triple::Simulator &&
+   (dyld_triple.getOS() == llvm::Triple::IOS ||
+dyld_triple.getOS() == llvm::Triple::TvOS ||
+dyld_triple.getOS() == llvm::Triple::WatchOS)))
 image_module_sp->MergeArchitecture(dyld_spec);
 }
   }
@@ -748,13 +753,23 @@ lldb_private::ArchSpec 
DynamicLoaderDarwin::ImageInfo::GetArchitecture() const {
   // Update the module's platform with the DYLD info.
   lldb_private::ArchSpec arch_spec(lldb_private::eArchTypeMachO, 
header.cputype,
header.cpusubtype);
-  if (os_type == llvm::Triple::IOS && os_env == llvm::Triple::MacABI) {
-llvm::Triple triple(llvm::Twine("x86_64-apple-ios") + min_version_os_sdk +
-"-macabi");
+  if (os_env == llvm::Triple::MacABI && os_type == llvm::Triple::IOS) {
+llvm::Triple triple(llvm::Twine(arch_spec.GetArchitectureName()) +
+"-apple-ios" + min_version_os_sdk + "-macabi");
 ArchSpec maccatalyst_spec(triple);
 if (arch_spec.IsCompatibleMatch(maccatalyst_spec))
   arch_spec.MergeFrom(maccatalyst_spec);
   }
+  if (os_env == llvm::Triple::Simulator &&
+  (os_type == llvm::Triple::IOS || os_type == llvm::Triple::TvOS ||
+   os_type == llvm::Triple::WatchOS)) {
+llvm::Triple triple(llvm::Twine(arch_spec.GetArchitectureName()) +
+"-apple-" + llvm::Triple::getOSTypeName(os_type) +
+min_version_os_sdk + "-simulator");
+ArchSpec sim_spec(triple);
+if (arch_spec.IsCompatibleMatch(sim_spec))
+  arch_spec.MergeFrom(sim_spec);
+  }
   return arch_spec;
 }
 

diff  --git a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py 
b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
index 4cc7ea6b8656..c12e920e3b82 100644
--- a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
+++ b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
@@ -12,10 +12,23 @@ class TestSimulatorPlatformLaunching(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-def run_with(self, arch, platform, os, env):
-self.build(dictionary={'TRIPLE': arch+'-apple-'+os+'-'+env, 'ARCH': 
arch})
+def check_load_commands(self, expected_load_command):
+"""sanity check the built binary for the expected number of load 
commands"""
+load_cmds = subprocess.check_output(
+['otool', '-l', self.getBuildArtifact()]
+).decode("utf-8")
+found = 0
+for line in load_cmds.split('\n'):
+if expected_load_command in line:
+  found += 1
+self.assertEquals(found, 1, "wrong load command")
+
+
+def run_with(self, arch, os, env, expected_load_command):
+self.build(dictionary={'TRIPLE': arch+'-apple-'+os+'-'+env})
 

[Lldb-commits] [PATCH] D82616: Improve the detection of iOS/tvOS/watchOS simulator binaries in debugserver and lldb

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0da0437b2afb: Improve the detection of iOS/tvOS/watchOS 
simulator binaries in debugserver and… (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D82616?vs=273569&id=274554#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82616

Files:
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -601,88 +601,78 @@
   plo_pthread_tsd_entry_size);
 }
 
-/// Determine whether this is running on macOS.
-/// Since debugserver runs on the same machine as the process, we can
-/// just look at the compilation target.
-static bool IsMacOSHost() {
-#if TARGET_OS_OSX == 1
-  return true;
-#else
-  return false;
-#endif
-}
-
-const char *MachProcess::GetDeploymentInfo(const struct load_command& lc,
-   uint64_t load_command_address,
-   uint32_t& major_version,
-   uint32_t& minor_version,
-   uint32_t& patch_version) {
+MachProcess::DeploymentInfo
+MachProcess::GetDeploymentInfo(const struct load_command &lc,
+   uint64_t load_command_address) {
+  DeploymentInfo info;
   uint32_t cmd = lc.cmd & ~LC_REQ_DYLD;
-  bool lc_cmd_known =
-cmd == LC_VERSION_MIN_IPHONEOS || cmd == LC_VERSION_MIN_MACOSX ||
-cmd == LC_VERSION_MIN_TVOS || cmd == LC_VERSION_MIN_WATCHOS;
-
-  if (lc_cmd_known) {
+  // Handle the older LC_VERSION load commands, which don't
+  // distinguish between simulator and real hardware.
+  auto handle_version_min = [&](char platform) {
 struct version_min_command vers_cmd;
 if (ReadMemory(load_command_address, sizeof(struct version_min_command),
-   &vers_cmd) != sizeof(struct version_min_command)) {
-  return nullptr;
-}
-major_version = vers_cmd.sdk >> 16;
-minor_version = (vers_cmd.sdk >> 8) & 0xffu;
-patch_version = vers_cmd.sdk & 0xffu;
-
-// Handle the older LC_VERSION load commands, which don't
-// distinguish between simulator and real hardware.
-switch (cmd) {
-case LC_VERSION_MIN_IPHONEOS:
-  return IsMacOSHost() ? "iossimulator": "ios";
-case LC_VERSION_MIN_MACOSX:
-  return "macosx";
-case LC_VERSION_MIN_TVOS:
-  return IsMacOSHost() ? "tvossimulator": "tvos";
-case LC_VERSION_MIN_WATCHOS:
-  return IsMacOSHost() ? "watchossimulator" : "watchos";
-default:
-  return nullptr;
-}
-  }
-#if defined (LC_BUILD_VERSION)
-  if (cmd == LC_BUILD_VERSION) {
+   &vers_cmd) != sizeof(struct version_min_command))
+  return;
+info.platform = platform;
+info.major_version = vers_cmd.sdk >> 16;
+info.minor_version = (vers_cmd.sdk >> 8) & 0xffu;
+info.patch_version = vers_cmd.sdk & 0xffu;
+info.maybe_simulator = true;
+  };
+  switch (cmd) {
+  case LC_VERSION_MIN_IPHONEOS:
+handle_version_min(PLATFORM_IOS);
+break;
+  case LC_VERSION_MIN_MACOSX:
+handle_version_min(PLATFORM_MACOS);
+break;
+  case LC_VERSION_MIN_TVOS:
+handle_version_min(PLATFORM_TVOS);
+break;
+  case LC_VERSION_MIN_WATCHOS:
+handle_version_min(PLATFORM_WATCHOS);
+break;
+#if defined(LC_BUILD_VERSION)
+  case LC_BUILD_VERSION: {
 struct build_version_command build_vers;
 if (ReadMemory(load_command_address, sizeof(struct build_version_command),
-   &build_vers) != sizeof(struct build_version_command)) {
-  return nullptr;
-}
-major_version = build_vers.sdk >> 16;;
-minor_version = (build_vers.sdk >> 8) & 0xffu;
-patch_version = build_vers.sdk & 0xffu;
-
-switch (build_vers.platform) {
-case PLATFORM_MACOS:
-  return "macosx";
-case PLATFORM_MACCATALYST:
-  return "maccatalyst";
-case PLATFORM_IOS:
-  return "ios";
-case PLATFORM_IOSSIMULATOR:
-  return "iossimulator";
-case PLATFORM_TVOS:
-  return "tvos";
-case PLATFORM_TVOSSIMULATOR:
-  return "tvossimulator";
-case PLATFORM_WATCHOS:
-  return "watchos";
-case PLATFORM_WATCHOSSIMULATOR:
-  return "watchossimulator";
-case PLATFORM_BRIDGEOS:
-  return "bridgeos";
-case PLATFORM_DRIVERKIT:
-  return "driverkit";
-}
+   &build_vers) != sizeof(struct build_version_command))
+  break;
+info.platfo

[Lldb-commits] [lldb] 66d9c0d - [lldb/Test] Skip TestIOHandlerResizeNoEditline with reproducers

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-30T11:40:17-07:00
New Revision: 66d9c0d4fc7ea19be08c6a3604571254186851a7

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

LOG: [lldb/Test] Skip TestIOHandlerResizeNoEditline with reproducers

Added: 


Modified: 
lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py

Removed: 




diff  --git a/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py 
b/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
index 4a99ff6d0ead..b0ef259fd85f 100644
--- a/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
+++ b/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
@@ -8,6 +8,7 @@ class TestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @no_debug_info_test
+@skipIfReproducer
 def test_resize_no_editline(self):
 """ Tests terminal resizing if the editline isn't used. """
 dbg = lldb.SBDebugger.Create(False)



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

[Lldb-commits] [PATCH] D82879: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 274561.
mib edited the summary of this revision.
mib added a comment.

Introduced `SBBreakpoint::AddNameWithError` to avoid API stability breakage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82879

Files:
  lldb/bindings/interface/SBBreakpoint.i
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/source/API/SBBreakpoint.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py

Index: lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -99,8 +99,8 @@
 other_bkpt_name = "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
-success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+success = bkpt.AddNameWithError(bkpt_name)
+self.assertTrue(success.Success(), "We couldn't add a legal name to a breakpoint.")
 
 matches = bkpt.MatchesName(bkpt_name)
 self.assertTrue(matches, "We didn't match the name we just set")
@@ -113,7 +113,7 @@
 self.check_name_in_target(bkpt_name)
 
 # Add another name, make sure that works too:
-bkpt.AddName(other_bkpt_name)
+bkpt.AddNameWithError(other_bkpt_name)
 
 matches = bkpt.MatchesName(bkpt_name)
 self.assertTrue(matches, "Adding a name means we didn't match the name we just set")
@@ -142,8 +142,8 @@
  "CantHave-ADash",
  "Cant Have Spaces"]
 for bad_name in bad_names:
-success = bkpt.AddName(bad_name)
-self.assertTrue(not success,"We allowed an illegal name: %s"%(bad_name))
+success = bkpt.AddNameWithError(bad_name)
+self.assertTrue(success.Fail(),"We allowed an illegal name: %s"%(bad_name))
 bp_name = lldb.SBBreakpointName(self.target, bad_name)
 self.assertFalse(bp_name.IsValid(), "We made a breakpoint name with an illegal name: %s"%(bad_name));
 
@@ -164,8 +164,8 @@
 other_bkpt_name= "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
-success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+success = bkpt.AddNameWithError(bkpt_name)
+self.assertTrue(success.Success(), "We couldn't add a legal name to a breakpoint.")
 
 bkpts = lldb.SBBreakpointList(self.target)
 self.target.FindBreakpointsByName(bkpt_name, bkpts)
@@ -243,8 +243,8 @@
 
 # Now add this name to a breakpoint, and make sure it gets configured properly
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
-success = bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+success = bkpt.AddNameWithError(self.bp_name_string)
+self.assertTrue(success.Success(), "Couldn't add this name to the breakpoint")
 self.check_option_values(bkpt)
 
 # Now make a name from this breakpoint, and make sure the new name is properly configured:
@@ -317,8 +317,8 @@
 unprotected_bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 unprotected_id = unprotected_bkpt.GetID()
 
-success = protected_bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+success = protected_bkpt.AddNameWithError(self.bp_name_string)
+self.assertTrue(success.Success(), "Couldn't add this name to the breakpoint")
 
 self.target.DisableAllBreakpoints()
 self.assertEqual(protected_bkpt.IsEnabled(), True, "Didnt' keep breakpoint from being disabled")
Index: lldb/source/API/SBBreakpoint.cpp
===
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -652,19 +652,28 @@
 bool SBBreakpoint::AddName(const char *new_name) {
   LLDB_RECORD_METHOD(bool, SBBreakpoint, AddName, (const char *), new_name);
 
+  SBError status = AddNameWithError(new_name);
+  return status.Success();
+}
+
+SBError SBBreakpoint::AddNameWithError(const char *new_name) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpoint, AddNameWithError, (const char *),
+ new_name);
+
   BreakpointSP bkpt_sp = GetSP();
 
+  SBError status;
   if (bkpt_sp) {
 std::lock_guard guard(
 bkpt_sp->GetTarget().GetAPIMutex());
-Status error; // Think I'm just going to swallow the error here, it's
-  // probably more annoying to have to provide it.
+Status error;
 bkpt_sp->GetTarget().AddNameToBreak

[Lldb-commits] [PATCH] D82879: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/bindings/interface/SBBreakpoint.i:210
+SBError
+AddNameWithError (const char *new_name);
+

In `SBDebugger` we have `InitializeWithErrorHandling`, maybe use the same 
suffix here for consistency? 



Comment at: 
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py:103
+success = bkpt.AddNameWithError(bkpt_name)
+self.assertTrue(success.Success(), "We couldn't add a legal name to a 
breakpoint.")
 

Seems like a good use case for Pavel's new `assertSuccess` helper? 
(https://reviews.llvm.org/D82759) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82879



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

[Lldb-commits] [lldb] 2501e86 - [lldb/Scalar] Fix undefined behavior

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-30T12:41:41-07:00
New Revision: 2501e86acda2905e50012f7e9fc1942517c1237d

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

LOG: [lldb/Scalar] Fix undefined behavior

Fix UBSan error detected in TestDataFormatterObjCCF.py and
TestDataFormatterObjCNSDate.py:

Scalar.cpp:698:27: runtime error: -4.96303e+08 is outside the range of
representable values of type 'unsigned long long'.

Added: 


Modified: 
lldb/source/Utility/Scalar.cpp

Removed: 




diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 610c935409ac..d275f6211e5c 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -736,7 +736,17 @@ long long Scalar::SLongLong(long long fail_value) const {
 }
 
 unsigned long long Scalar::ULongLong(unsigned long long fail_value) const {
-  return GetAsUnsigned(fail_value);
+  switch (m_type) {
+  case e_double: {
+double d_val = m_float.convertToDouble();
+llvm::APInt rounded_double =
+llvm::APIntOps::RoundDoubleToAPInt(d_val, sizeof(ulonglong_t) * 8);
+return static_cast(
+(rounded_double.zextOrTrunc(sizeof(ulonglong_t) * 8)).getZExtValue());
+  }
+  default:
+return GetAsUnsigned(fail_value);
+  }
 }
 
 llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const {



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

Re: [Lldb-commits] [lldb] 2501e86 - [lldb/Scalar] Fix undefined behavior

[Jonas Devlieghere via lldb-commits]

+ Pavel for post-commit review.

I've reinstated the original behavior to fix the UBSan error on the bot.
Please let me know if you prefer a different approach.

On Tue, Jun 30, 2020 at 12:41 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Jonas Devlieghere
> Date: 2020-06-30T12:41:41-07:00
> New Revision: 2501e86acda2905e50012f7e9fc1942517c1237d
>
> URL:
> https://github.com/llvm/llvm-project/commit/2501e86acda2905e50012f7e9fc1942517c1237d
> DIFF:
> https://github.com/llvm/llvm-project/commit/2501e86acda2905e50012f7e9fc1942517c1237d.diff
>
> LOG: [lldb/Scalar] Fix undefined behavior
>
> Fix UBSan error detected in TestDataFormatterObjCCF.py and
> TestDataFormatterObjCNSDate.py:
>
> Scalar.cpp:698:27: runtime error: -4.96303e+08 is outside the range of
> representable values of type 'unsigned long long'.
>
> Added:
>
>
> Modified:
> lldb/source/Utility/Scalar.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/source/Utility/Scalar.cpp
> b/lldb/source/Utility/Scalar.cpp
> index 610c935409ac..d275f6211e5c 100644
> --- a/lldb/source/Utility/Scalar.cpp
> +++ b/lldb/source/Utility/Scalar.cpp
> @@ -736,7 +736,17 @@ long long Scalar::SLongLong(long long fail_value)
> const {
>  }
>
>  unsigned long long Scalar::ULongLong(unsigned long long fail_value) const
> {
> -  return GetAsUnsigned(fail_value);
> +  switch (m_type) {
> +  case e_double: {
> +double d_val = m_float.convertToDouble();
> +llvm::APInt rounded_double =
> +llvm::APIntOps::RoundDoubleToAPInt(d_val, sizeof(ulonglong_t) *
> 8);
> +return static_cast(
> +(rounded_double.zextOrTrunc(sizeof(ulonglong_t) *
> 8)).getZExtValue());
> +  }
> +  default:
> +return GetAsUnsigned(fail_value);
> +  }
>  }
>
>  llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const {
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4f1f253 - [TestReturnValue] Skip based on architecutre, not platform.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-06-30T12:48:23-07:00
New Revision: 4f1f253ce509ebc20e1f7a2d8bb6885366b155f2

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

LOG: [TestReturnValue] Skip based on architecutre, not platform.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/test/API/functionalities/return-value/TestReturnValue.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 0ef2f931542c..ec084184cd65 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -411,14 +411,15 @@ def expectedFailureOS(
 debug_info=debug_info)
 
 
-def expectedFailureDarwin(bugnumber=None, compilers=None, debug_info=None):
+def expectedFailureDarwin(bugnumber=None, compilers=None, debug_info=None, 
archs=None):
 # For legacy reasons, we support both "darwin" and "macosx" as OS X
 # triples.
 return expectedFailureOS(
 lldbplatform.darwin_all,
 bugnumber,
 compilers,
-debug_info=debug_info)
+debug_info=debug_info,
+archs=archs)
 
 
 def expectedFailureAndroid(bugnumber=None, api_levels=None, archs=None):

diff  --git a/lldb/test/API/functionalities/return-value/TestReturnValue.py 
b/lldb/test/API/functionalities/return-value/TestReturnValue.py
index 24aa504a593c..08293ca48b95 100644
--- a/lldb/test/API/functionalities/return-value/TestReturnValue.py
+++ b/lldb/test/API/functionalities/return-value/TestReturnValue.py
@@ -165,7 +165,7 @@ def test_with_python(self):
 archs=["i386"])
 @expectedFailureAll(compiler=["gcc"], archs=["x86_64", "i386"])
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
-@skipIfDarwinEmbedded #  ABIMacOSX_arm64 doesn't 
get structs this big correctly
+@expectedFailureDarwin(archs=["arm64"]) #  
ABIMacOSX_arm64 doesn't get structs this big correctly
 def test_vector_values(self):
 self.build()
 exe = self.getBuildArtifact("a.out")



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

[Lldb-commits] [lldb] 983a4b5 - Skip arm-(fp|gp)-read.test on Darwin.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-06-30T12:53:20-07:00
New Revision: 983a4b520ea384ac978cbfc40bb956fbb98cd735

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

LOG: Skip arm-(fp|gp)-read.test on Darwin.

Our assembler doesn't seem to grok floating point literals.



Added: 


Modified: 
lldb/test/Shell/Register/arm-fp-read.test
lldb/test/Shell/Register/arm-gp-read.test

Removed: 




diff  --git a/lldb/test/Shell/Register/arm-fp-read.test 
b/lldb/test/Shell/Register/arm-fp-read.test
index 538d6af54dcf..116fa7aa0df5 100644
--- a/lldb/test/Shell/Register/arm-fp-read.test
+++ b/lldb/test/Shell/Register/arm-fp-read.test
@@ -1,4 +1,4 @@
-# REQUIRES: native && target-arm
+# REQUIRES: native && target-arm && !system-darwin
 # RUN: %clangxx_host -fomit-frame-pointer %p/Inputs/arm-fp-read.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch

diff  --git a/lldb/test/Shell/Register/arm-gp-read.test 
b/lldb/test/Shell/Register/arm-gp-read.test
index bcb289b880a7..9f8461842e9b 100644
--- a/lldb/test/Shell/Register/arm-gp-read.test
+++ b/lldb/test/Shell/Register/arm-gp-read.test
@@ -1,4 +1,4 @@
-# REQUIRES: native && target-arm
+# REQUIRES: native && target-arm && !system-darwin
 # RUN: %clangxx_host -fomit-frame-pointer %p/Inputs/arm-gp-read.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Host/Host.h:231
 
+  /// Check whether a process is translated (Rosetta).
+  /// \arg process_info The info structure for the process queried.

davide wrote:
> aprantl wrote:
> > Is this supposed to be a generic function call that, e.g, should fire for 
> > debugging a process in QEMU under Linux, or is it meant to specifically 
> > check for Rosetta on macOS? The comment makes it sound like it's the latter 
> > and the API name hints at the former. It would be nice to clarify this in 
> > the comment.
> It is meant specifically for Rosetta, but this leaves in `Host`, so I decided 
> for a generic name.
> Do you have a better suggestion for the name or the comment ? Happy to go 
> with it 
Not awesome, but `IsMacOSRosettaProcess` or `IsMacOSRosettaTranslated`?

A more serious question: Is Host actually the right place for this function? 
What happens when I remote-debug a Rosetta process from another Mac or a Linux 
machine? Is there even a correct abstraction for this use-case? Platform maybe?


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Yifan Shen via Phabricator via lldb-commits]

aelitashen marked 3 inline comments as done.
aelitashen added inline comments.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:337-340
+std::string(module.GetFileSpec().GetFilename()));
+  std::string module_path = std::string(module.GetFileSpec().GetDirectory()) +
+"/" +
+std::string(module.GetFileSpec().GetFilename());

clayborg wrote:
> Let LLDB take care of the directory delimiter. Otherwise this will be bad on 
> windows:
> ```
> char module_path[PATH_MAX];
> module.GetFileSpec().GetPath(module_path, sizeof(module_path));
> ```
> 
This piece of code will make path unreadable in the log, and cause the modules 
view cannot be displayed properly.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:344-346
+std::string symbol_path =
+std::string(module.GetSymbolFileSpec().GetDirectory()) + "/" +
+std::string(module.GetSymbolFileSpec().GetFilename());

clayborg wrote:
> Let LLDB take care of the directory delimiter. Otherwise this will be bad on 
> windows:
> 
> ```
> char symbol_path[PATH_MAX];
> module.GetSymbolFileSpec().GetPath(module_path, sizeof(symbol_path));
> ```
> 
Same reason as above.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:349-350
+  }
+  std::string loaded_addr = std::to_string(
+  module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target));
+  object.try_emplace("addressRange", loaded_addr);

clayborg wrote:
> We need to make sure the address returned is valid and we want the string 
> value to be in hex. Are we sure std::to_string() will create a hex number? If 
> not we can use sprintf:
> 
> ```
> uint64_t load_addr = 
> module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target);
> if (load_addr != LLDB_INVALID_ADDRESS) {
>   char load_addr_str[64];
>   snprintf(load_addr_str, sizeof(load_addr_str), "0x%016" PRIx64, load_addr);
>   object.try_emplace("addressRange", load_addr_str);
> }
> ```
Decimal to Hex conversion is done in the IDE, not sure if we need to move it 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477



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

[Lldb-commits] [PATCH] D82732: WIP: Use SB APIs in CommandObjects

[Shivam Mittal via Phabricator via lldb-commits]

shivammittal99 updated this revision to Diff 274625.
shivammittal99 added a comment.

- Extend SBTarget to enable/disable/ignore watchpoints


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82732

Files:
  lldb/bindings/interface/SBTarget.i
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/include/lldb/API/SBBreakpointLocation.h
  lldb/include/lldb/API/SBTarget.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpoint.h
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp

Index: lldb/source/Commands/CommandObjectWatchpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -19,6 +19,9 @@
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Target/Target.h"
 
+#include "lldb/API/SBTarget.h"
+#include "lldb/API/SBWatchpoint.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -398,10 +401,10 @@
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-Target *target = &GetSelectedTarget();
+TargetSP target_sp = GetDebugger().GetSelectedTarget();
+lldb::SBTarget sb_target(target_sp);
 
-const WatchpointList &watchpoints = target->GetWatchpointList();
-size_t num_watchpoints = watchpoints.GetSize();
+size_t num_watchpoints = sb_target.GetNumWatchpoints();
 
 if (num_watchpoints == 0) {
   result.AppendError("No watchpoints exist to have commands added");
@@ -417,8 +420,8 @@
 }
 
 std::vector valid_wp_ids;
-if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(target, command,
-   valid_wp_ids)) {
+if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(
+sb_target, command, valid_wp_ids)) {
   result.AppendError("Invalid watchpoints specification.");
   result.SetStatus(eReturnStatusFailed);
   return false;
@@ -429,12 +432,12 @@
 for (size_t i = 0; i < count; ++i) {
   uint32_t cur_wp_id = valid_wp_ids.at(i);
   if (cur_wp_id != LLDB_INVALID_WATCH_ID) {
-Watchpoint *wp = target->GetWatchpointList().FindByID(cur_wp_id).get();
+lldb::SBWatchpoint sb_wp = sb_target.FindWatchpointByID(cur_wp_id);
 // Sanity check wp first.
-if (wp == nullptr)
+if (!sb_wp)
   continue;
 
-WatchpointOptions *wp_options = wp->GetOptions();
+WatchpointOptions *wp_options = sb_wp.GetSP()->GetOptions();
 // Skip this watchpoint if wp_options is not good.
 if (wp_options == nullptr)
   continue;
@@ -508,10 +511,10 @@
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-Target *target = &GetSelectedTarget();
+TargetSP target_sp = GetDebugger().GetSelectedTarget();
+lldb::SBTarget sb_target(target_sp);
 
-const WatchpointList &watchpoints = target->GetWatchpointList();
-size_t num_watchpoints = watchpoints.GetSize();
+size_t num_watchpoints = sb_target.GetNumWatchpoints();
 
 if (num_watchpoints == 0) {
   result.AppendError("No watchpoints exist to have commands deleted");
@@ -527,8 +530,8 @@
 }
 
 std::vector valid_wp_ids;
-if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(target, command,
-   valid_wp_ids)) {
+if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(
+sb_target, command, valid_wp_ids)) {
   result.AppendError("Invalid watchpoints specification.");
   result.SetStatus(eReturnStatusFailed);
   return false;
@@ -539,9 +542,9 @@
 for (size_t i = 0; i < count; ++i) {
   uint32_t cur_wp_id = valid_wp_ids.at(i);
   if (cur_wp_id != LLDB_INVALID_WATCH_ID) {
-Watchpoint *wp = target->GetWatchpointList().FindByID(cur_wp_id).get();
-if (wp)
-  wp->ClearCallback();
+SBWatchpoint sb_wp = sb_target.FindWatchpointByID(cur_wp_id);
+if (sb_wp)
+  sb_wp.GetSP()->ClearCallback();
   } else {
 result.AppendErrorWithFormat("Invalid watchpoint ID: %u.\n", cur_wp_id);
 result.SetStatus(eReturnStatusFailed);
@@ -580,10 +583,10 @@
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-Target *target = &GetSelectedTarget();
+TargetSP target_sp = GetDebugger().GetSelectedTarget();
+lldb::SBTarget sb_target(target_sp);
 
-const WatchpointList &watchpoints = target->GetWatchpointList();
-size_t num_watchpoints = watchpoints.GetSize();
+size_t num_watchpoints = sb_target.GetNumWatchpoints();
 
 if (num_watchpoints == 0) {
   result.AppendE

[Lldb-commits] [PATCH] D82879: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 274633.
mib added a comment.

- Use new `assertSuccess` method in test.
- Rename the method to `AddNameWithErrorHandling` for consistency.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82879

Files:
  lldb/bindings/interface/SBBreakpoint.i
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/source/API/SBBreakpoint.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py

Index: lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -99,8 +99,8 @@
 other_bkpt_name = "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
-success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+success = bkpt.AddNameWithErrorHandling(bkpt_name)
+self.assertSuccess(success, "We couldn't add a legal name to a breakpoint.")
 
 matches = bkpt.MatchesName(bkpt_name)
 self.assertTrue(matches, "We didn't match the name we just set")
@@ -113,7 +113,7 @@
 self.check_name_in_target(bkpt_name)
 
 # Add another name, make sure that works too:
-bkpt.AddName(other_bkpt_name)
+bkpt.AddNameWithErrorHandling(other_bkpt_name)
 
 matches = bkpt.MatchesName(bkpt_name)
 self.assertTrue(matches, "Adding a name means we didn't match the name we just set")
@@ -142,8 +142,8 @@
  "CantHave-ADash",
  "Cant Have Spaces"]
 for bad_name in bad_names:
-success = bkpt.AddName(bad_name)
-self.assertTrue(not success,"We allowed an illegal name: %s"%(bad_name))
+success = bkpt.AddNameWithErrorHandling(bad_name)
+self.assertTrue(success.Fail(),"We allowed an illegal name: %s"%(bad_name))
 bp_name = lldb.SBBreakpointName(self.target, bad_name)
 self.assertFalse(bp_name.IsValid(), "We made a breakpoint name with an illegal name: %s"%(bad_name));
 
@@ -164,8 +164,8 @@
 other_bkpt_name= "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
-success = bkpt.AddName(bkpt_name)
-self.assertTrue(success, "We couldn't add a legal name to a breakpoint.")
+success = bkpt.AddNameWithErrorHandling(bkpt_name)
+self.assertSuccess(success, "We couldn't add a legal name to a breakpoint.")
 
 bkpts = lldb.SBBreakpointList(self.target)
 self.target.FindBreakpointsByName(bkpt_name, bkpts)
@@ -243,8 +243,8 @@
 
 # Now add this name to a breakpoint, and make sure it gets configured properly
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
-success = bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+success = bkpt.AddNameWithErrorHandling(self.bp_name_string)
+self.assertSuccess(success, "Couldn't add this name to the breakpoint")
 self.check_option_values(bkpt)
 
 # Now make a name from this breakpoint, and make sure the new name is properly configured:
@@ -317,8 +317,8 @@
 unprotected_bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 unprotected_id = unprotected_bkpt.GetID()
 
-success = protected_bkpt.AddName(self.bp_name_string)
-self.assertTrue(success, "Couldn't add this name to the breakpoint")
+success = protected_bkpt.AddNameWithErrorHandling(self.bp_name_string)
+self.assertSuccess(success, "Couldn't add this name to the breakpoint")
 
 self.target.DisableAllBreakpoints()
 self.assertEqual(protected_bkpt.IsEnabled(), True, "Didnt' keep breakpoint from being disabled")
Index: lldb/source/API/SBBreakpoint.cpp
===
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -652,19 +652,28 @@
 bool SBBreakpoint::AddName(const char *new_name) {
   LLDB_RECORD_METHOD(bool, SBBreakpoint, AddName, (const char *), new_name);
 
+  SBError status = AddNameWithErrorHandling(new_name);
+  return status.Success();
+}
+
+SBError SBBreakpoint::AddNameWithErrorHandling(const char *new_name) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpoint, AddNameWithErrorHandling,
+ (const char *), new_name);
+
   BreakpointSP bkpt_sp = GetSP();
 
+  SBError status;
   if (bkpt_sp) {
 std::lock_guard guard(
 bkpt_sp->GetTarget().GetAPIMutex());
-Status error; // Think I'm just going to swallow the error here, it's
-  // probably more annoying to have to provide it.
+Status error;
   

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 274634.
aelitashen added a comment.

Make LLDB take care of the directory delimiter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/module/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
@@ -433,6 +434,30 @@
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
+  } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
+event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
+event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+  for (int i = 0; i < num_modules; i++) {
+auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+auto module_event = CreateEventObject("module");
+llvm::json::Value module_value = CreateModule(module);
+llvm::json::Object body;
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
+  body.try_emplace("reason", "new");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+  body.try_emplace("reason", "removed");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  body.try_emplace("reason", "changed");
+}
+body.try_emplace("module", module_value);
+module_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
+  }
+}
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
 if (event_mask & eBroadcastBitStopEventThread) {
   done = true;
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -354,6 +354,11 @@
 lldb::SBTarget::eBroadcastBitBreakpointChanged);
 listener.StartListeningForEvents(this->broadcaster,
  eBroadcastBitStopEventThread);
+listener.StartListeningForEvents(
+  this->target.GetBroadcaster(),
+  lldb::SBTarget::eBroadcastBitModulesLoaded |
+  lldb::SBTarget::eBroadcastBitModulesUnloaded |
+  lldb::SBTarget::eBroadcastBitSymbolsLoaded);
   }
 }
 
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "VSCodeForward.h"
+#include "lldb/API/SBModule.h"
 
 namespace lldb_vscode {
 
@@ -237,6 +238,16 @@
  llvm::Optional request_path = llvm::None,
  llvm::Optional request_line = llvm::None);
 
+/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
+///
+/// \param[in] module
+/// A LLDB module object to convert into a JSON value
+///
+/// \return
+/// A "Module" JSON object with that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateModule(lldb::SBModule &module);
+
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
 /// \param[in] event_name
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,29 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateModule(lldb::SBModule &module) {
+  llvm::json::Object object;
+  if (!module.IsValid())
+return llvm::json::Value(std::move(object));
+  object.try_emplace("id", std::string(module.GetUUIDString()));
+  object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
+  char module_path_arr[PATH_MAX];
+  module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
+  std::string module_path(module_path_arr);
+ 

[Lldb-commits] [PATCH] D82835: [lldb] Fix missing characters when autocompleting LLDB commands in REPL

[Martin Svensson via Phabricator via lldb-commits]

poya marked an inline comment as done.
poya added inline comments.



Comment at: lldb/source/Expression/REPL.cpp:460
 sub_result.GetMatches(matches);
+// Prepend command prefix that was excluded in the completion request
+if (request.GetCursorIndex() == 0) {

teemperor wrote:
> Nit: Missing `.` at the end of the comment, but I'll fix that when landing.
Thanks, I wasn't sure about the convention since other comments in the same 
file had it both ways


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82835



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Host/Host.h:231
 
+  /// Check whether a process is translated (Rosetta).
+  /// \arg process_info The info structure for the process queried.

aprantl wrote:
> davide wrote:
> > aprantl wrote:
> > > Is this supposed to be a generic function call that, e.g, should fire for 
> > > debugging a process in QEMU under Linux, or is it meant to specifically 
> > > check for Rosetta on macOS? The comment makes it sound like it's the 
> > > latter and the API name hints at the former. It would be nice to clarify 
> > > this in the comment.
> > It is meant specifically for Rosetta, but this leaves in `Host`, so I 
> > decided for a generic name.
> > Do you have a better suggestion for the name or the comment ? Happy to go 
> > with it 
> Not awesome, but `IsMacOSRosettaProcess` or `IsMacOSRosettaTranslated`?
> 
> A more serious question: Is Host actually the right place for this function? 
> What happens when I remote-debug a Rosetta process from another Mac or a 
> Linux machine? Is there even a correct abstraction for this use-case? 
> Platform maybe?
This code is only used when debugging on the host. When debugging remotely, you 
either connect to at already running instance of debugserver, or you ask the 
lldb-server-platform process to launch one for you. The platform process then 
should use this function to find the right debugserver to launch, but using a 
Host function is appropriate there, because the platform process is running 
remotely, and so it will the the "remote Host" which is answering the question.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);

davide wrote:
> labath wrote:
> > I don't think this is a particularly good abstraction, as the debugserver 
> > path is still left here, and the path is definitely os- and arch-specific. 
> > It would be better if the API returned the path to the 
> > debugserver-to-be-used instead.
> > 
> > Bonus points if you can also hide the `DEBUGSERVER_BASENAME` logic from 
> > GDBRemoteCommunication.cpp into the same API.
> hmm, I wonder if this code should live in `GDBRemoteCommunication.cpp`
I think it ended up there because the gdb-server launching code is used from 
both liblldb and lldb-server (the platform version), and GDBRemoteCommunication 
is the greatest common denominator. It's not ideal, but I don't think its the 
worst thing that we have. (At one point I'd like to create a library to house 
code which is shared between liblldb and lldb-server, and then it may be easier 
to organize things better.)

Speaking of that... I think this code should live in GDBRemoteCommunication as 
well. In the situation where one is remote-debugging a mac via 
lldb-server-platform, I assume one would want the platform process to 
automatically spawn the right debugserver for the rosetta thingies.


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

https://reviews.llvm.org/D82813



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