[Lldb-commits] [PATCH] D132307: [lldb] Switch RegularExpression from llvm::Regex to std::regex

2022-08-22 Thread Pavel Labath via Phabricator via lldb-commits 
labath requested changes to this revision.
labath added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Utility/CMakeLists.txt:29
+  PROPERTIES COMPILE_OPTIONS
+  "-fcxx-exceptions"
+)

kastiglione wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > kastiglione wrote:
> > > > the `std::regex` constructor throws `std::regex_error` if the pattern 
> > > > is invalid. For this reason, exceptions are enabled for this one file.
> > > What happens when exceptions are disabled? What does it mean to have this 
> > > enabled for a single file? I don't know if it's part of the LLVM 
> > > developer guide, but LLVM is supposed to build without RTTI and without 
> > > exceptions. 
> > > What happens when exceptions are disabled?
> > 
> > This cmake config enables exceptions for this one file, independent of 
> > `LLVM_ENABLE_EH`. No other source files will be allowed to catch or throw 
> > exceptions.
> > 
> > > What does it mean to have this enabled for a single file?
> > 
> > It means this file can compile with a try/catch, and that inside this file, 
> > exceptions can be caught.
> > 
> > > I don't know if it's part of the LLVM developer guide, but LLVM is 
> > > supposed to build without RTTI and without exceptions.
> > 
> > llvm has `LLVM_ENABLE_EH` which allows llvm to be built with exceptions 
> > support enabled. Similarly, `LLVM_ENABLE_RTTI` allows RTTI to be enabled. 
> > It seems that both are disabled as a default, but not as a hard requirement.
> > 
> > I wondered if enabling RTTI would needed for exceptions, but at least for 
> > this code, the answer is no. The `catch (const std::regex_error &e)` block 
> > is exercised by `TestBreakpointRegexError.py`, so we know this code can and 
> > does catch an exception of that type, and accesses the error's member 
> > functions.
> > 
> > 
> What makes me believe this use of exceptions is ok is that the API boundary 
> continues to be exception free, callers continue to be exception disabled. 
> Only the internal implementation, knows about exceptions.
That is definitely not ok. ODR madness awaits therein. The standard library is 
full of functions that effectively do
```
#ifdef EXCEPTIONS
  something();
#else
  something_else();
#endif
```
Compiling just one file with exceptions enabled can cause two different 
versions of those functions to appear. It is sufficient that this file 
instantiates one function whose behavior is dependent on exceptions being 
available. When linking, the linker has to choose one of the two versions, and 
there's no telling which one will it use. This means that exceptions can creep 
in into the supposedly exception-free code, or vice-versa.

The only way this would be safe is if this code was in a shared library, and we 
took care to restrict the visibility of all symbols except the exported api of 
that library. And I don't think that's worth it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132307

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

[Lldb-commits] [PATCH] D132231: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

2022-08-22 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yeah, fixing this correctly is not going to be trivial. Off the top of my head, 
I can think of two solutions (creating per-file ASTs for static objects, or 
putting all static objects from a given file into a special namespace) and each 
of them has a lot of drawbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132231

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

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

2022-08-22 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D132217#3735180 , @mgorny wrote:

> @labath, any chance you could take a look at the failures on Windows? I'm 
> wondering if this is because "pipes don't work like that on Windows" or if 
> there's a way to make them work. For now I'm just going to skip these tests 
> on win32.
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/22672

I'm not sure why the first write fails, but I believe that even if we fixed 
that, that we would have problems with the read operation, as the pipe FD will 
not work with select. I think the safest/simplest way to fix this would be to 
change the test to use sockets instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132217

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

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

2022-08-22 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

What's the reasoning behind `TriggerPendingCallbacks`? I was assuming that the 
addition of a callback would cause it to run automatically...


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

https://reviews.llvm.org/D131160

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

[Lldb-commits] [PATCH] D132191: Treat a UUID of all zeros consistently to mean "Invalid UUID"

2022-08-22 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added inline comments.



Comment at: lldb/include/lldb/Utility/UUID.h:47
+  /// Creates a UUID from the data pointed to by the bytes argument. 
   static UUID fromData(const void *bytes, uint32_t num_bytes) {
 if (bytes)

The main reason this was a factory function was so that we could use the 
factory name to disambiguate between the two interpretations of zero "uuids".

As we're removing that, we could also go back to using regular constructors for 
object creation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132191

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

[Lldb-commits] [PATCH] D132231: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

2022-08-22 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 454406.
Michael137 added a comment.

- Move one passing test case out of xfail


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132231

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -36,6 +36,55 @@
 substrs=['stopped',
  'stop reason = breakpoint'])
 
+@skipIfWindows # This is flakey on Windows: llvm.org/pr38373
+@expectedFailure("CU-local objects incorrectly scoped")
+def test_scope_lookup_with_run_command_globals(self):
+"""Test scope lookup of functions in lldb."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self,
+self.line_break_global_scope,
+lldb.SBFileSpec("ns.cpp"))
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"ns3.cpp",
+self.line_break_before_using_directive,
+num_expected_locations=1,
+loc_exact=False)
+
+# Run to BP_global_scope at file scope
+self.runToBkpt("run")
+
+# FIXME: LLDB does not correctly scope CU-local objects.
+# LLDB currently lumps functions from all files into
+# a single AST and depending on the order with which
+# functions are considered, LLDB can incorrectly call
+# the static local ns.cpp::func() instead of the expected
+# ::func()
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", expect_type="int", expect_value="1")
+
+# Evaluate ::func() - should call A::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_before_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", result_type="int", result_value="1")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_after_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_with_run_command(self):
 """Test scope lookup of functions in lldb."""
@@ -66,6 +115,12 @@
 self.line_break_nested_ns_scope_after_using,
 num_expected_locations=1,
 loc_exact=False)
+lldbutil.run_break_set_by_file_and_line(
+self,
+"ns2.cpp",
+self.line_break_file_scope,
+num_expected_locations=1,
+loc_exact=False)
 lldbutil.run_break_set_by_file_and_line(
 self,
 "ns3.cpp",
@@ -81,17 +136,18 @@
 
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
-# Evaluate func() - should call ::func()
-self.expect_expr("func()", result_type="int", result_value="1")
+
 # Evaluate A::B::func() - should call A::B::func()
 self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
 self.expect_expr("func(10)", result_type="int", result_value="11")
-# Evaluate ::func() - should call A::func()
-self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
 self.expect_expr("A::foo()", result_type="int", result_value="42")
 
+# Continue to BP_file_scope at file scope
+self.runToBkpt("continue")
+self.expect_expr("func()", result_type="int", result_value="2")
+
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
@@ -124,16 +180,12 @@
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
-# Evaluate ::func() - should call ::func()
-self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
 self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # declaration
 self.runToBkpt("continue")
-# Evaluate ::func() - should call ::func()
-self.expect_expr("::func()", result_type="int

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

2022-08-22 Thread Emmmer S via Phabricator via lldb-commits 
Emmmer created this revision.
Herald added subscribers: atanasyan, jrtc27, nemanjai, emaste.
Herald added a project: All.
Emmmer edited the summary of this revision.
Emmmer added reviewers: JDevlieghere, DavidSpickett.
Emmmer added a project: LLDB.
Emmmer added subscribers: imkiva, liaolucy.
Emmmer published this revision for review.
Herald added a subscriber: lldb-commits.

Using GetTargetArchitecture() instead of m_target_arch, and set m_target_arch 
to private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132353

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
@@ -61,9 +61,9 @@
 return nullptr;
   }
 
-  // FIXME make private.
+private:
   lldb_private::ArchSpec m_target_arch;
 };
-}
+} // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
@@ -399,7 +399,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 m_reg_info.num_registers = k_num_registers_i386;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_i386;
@@ -518,7 +518,7 @@
 
 const RegisterSet *RegisterContextPOSIX_x86::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::x86:
   return &g_reg_sets_i386[set];
 case llvm::Triple::x86_64:
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
@@ -83,7 +83,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::systemz:
 m_reg_info.num_registers = k_num_registers_s390x;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_s390x;
@@ -151,7 +151,7 @@
 
 const RegisterSet *RegisterContextPOSIX_s390x::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::systemz:
   return &g_reg_sets_s390x[set];
 default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
@@ -62,7 +62,7 @@
 size_t RegisterContextOpenBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextOpenBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 return g_register_infos_i386;
   default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
@@ -80,7 +80,7 @@
 size_t RegisterContextNetBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextNetBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 return g_register_infos_i386;
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux

[Lldb-commits] [PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

2022-08-22 Thread Sebastian Neubauer via Phabricator via lldb-commits 
sebastian-ne added inline comments.



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:89
 
-  set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports)
 

The lib here is not used as a folder, so this should probably not be replaced?
It should probably include CMAKE_CFG_INTDIR though (i.e. 
`${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/libllvm-c.exports`), to be consistent 
with other places, though this only affects the ninja multi-config build. I’m 
not sure if that works at all and it’s outside of the scope of this patch 
anyway.



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:91
 
   set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)

Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:139
   foreach(lib ${LIB_NAMES})
 list(APPEND FULL_LIB_NAMES 
${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
   endforeach()

This should probably use LLVM_LIBRARY_DIR as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

2022-08-22 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp:227
 const RegisterInfo *RegisterContextFreeBSD_powerpc64::GetRegisterInfo() const {
   // assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
+  if (GetTargetArchitecture().GetMachine() == llvm::Triple::ppc)

Please also remove the commented out asserts that use `m_target_arch`.

At least for PowerPC they have been commented out since the first commit so it 
won't do any harm to remove them and they will now fail to compile anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132353

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

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

2022-08-22 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h:64
 
-  // FIXME make private.
   lldb_private::ArchSpec m_target_arch;

Rare footage of a fixme getting fixed :) Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132353

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

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

2022-08-22 Thread Emmmer S via Phabricator via lldb-commits 
Emmmer updated this revision to Diff 454426.
Emmmer added a comment.

Address review comments


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

https://reviews.llvm.org/D132353

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
@@ -61,9 +61,9 @@
 return nullptr;
   }
 
-  // FIXME make private.
+private:
   lldb_private::ArchSpec m_target_arch;
 };
-}
+} // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
@@ -399,7 +399,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 m_reg_info.num_registers = k_num_registers_i386;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_i386;
@@ -518,7 +518,7 @@
 
 const RegisterSet *RegisterContextPOSIX_x86::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::x86:
   return &g_reg_sets_i386[set];
 case llvm::Triple::x86_64:
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
@@ -83,7 +83,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::systemz:
 m_reg_info.num_registers = k_num_registers_s390x;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_s390x;
@@ -151,7 +151,7 @@
 
 const RegisterSet *RegisterContextPOSIX_s390x::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::systemz:
   return &g_reg_sets_s390x[set];
 default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
@@ -62,7 +62,7 @@
 size_t RegisterContextOpenBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextOpenBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 return g_register_infos_i386;
   default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
@@ -80,7 +80,7 @@
 size_t RegisterContextNetBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextNetBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 return g_register_infos_i386;
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
@@ -107,7 +107,7 @@
 size_t RegisterContextLinux_i386::GetGPRSizeStatic() { return sizeof(GPR); }
 
 const RegisterInfo *Register

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

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

I think there are a few more commented out asserts, check that a grep for 
`m_target_arch` in `lldb/` doesn't turn up anything you wouldn't expect.

Otherwise LGTM. If you find any more just land it with them removed.


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

https://reviews.llvm.org/D132353

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

[Lldb-commits] [lldb] 65f6a8c - [LLDB] Fix: make m_target_arch private

2022-08-22 Thread via lldb-commits 

Author: Emmmer
Date: 2022-08-22T18:01:44+08:00
New Revision: 65f6a8c23b5315628c980149be73e692be8e2aa6

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

LOG: [LLDB] Fix: make m_target_arch private

Using GetTargetArchitecture() instead of m_target_arch, and set m_target_arch 
to private.

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
index acebe9d535687..df6a82c11255e 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -68,7 +68,7 @@ RegisterContextFreeBSD_i386::RegisterContextFreeBSD_i386(
 size_t RegisterContextFreeBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextFreeBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 return g_register_infos_i386;
   default:

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
index 7b4c7be21f747..1f52c09df12e7 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
@@ -169,7 +169,7 @@ size_t RegisterContextFreeBSD_mips64::GetRegisterSetCount() 
const {
 }
 
 const RegisterInfo *RegisterContextFreeBSD_mips64::GetRegisterInfo() const {
-  assert(m_target_arch.GetCore() == ArchSpec::eCore_mips64);
+  assert(GetTargetArchitecture().GetCore() == ArchSpec::eCore_mips64);
   return g_register_infos_mips64;
 }
 

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
index 2991bd3c5f2cf..d8dfa434335be 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
@@ -186,7 +186,6 @@ size_t RegisterContextFreeBSD_powerpc::GetGPRSize() const {
 }
 
 const RegisterInfo *RegisterContextFreeBSD_powerpc::GetRegisterInfo() const {
-  // assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
   llvm_unreachable("Abstract class!");
   return nullptr;
 }
@@ -204,7 +203,6 @@ size_t RegisterContextFreeBSD_powerpc32::GetGPRSize() const 
{
 }
 
 const RegisterInfo *RegisterContextFreeBSD_powerpc32::GetRegisterInfo() const {
-  // assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
   return g_register_infos_powerpc32;
 }
 
@@ -224,8 +222,7 @@ size_t RegisterContextFreeBSD_powerpc64::GetGPRSize() const 
{
 }
 
 const RegisterInfo *RegisterContextFreeBSD_powerpc64::GetRegisterInfo() const {
-  // assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
-  if (m_target_arch.GetMachine() == llvm::Triple::ppc)
+  if (GetTargetArchitecture().GetMachine() == llvm::Triple::ppc)
 return g_register_infos_powerpc64_32;
   return g_register_infos_powerpc64;
 }

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
index bb3bb01bdf02c..0b3953e1ec2c1 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
@@ -107,7 +107,7 @@ RegisterContextLinux_i386::RegisterContextLinux_i386(
 size_t RegisterContextLinux_i386::GetGPRSizeStatic() { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextLinux_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 return g_register_infos_i386;

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
index b

[Lldb-commits] [PATCH] D132353: [LLDB] Fix: make m_target_arch private

2022-08-22 Thread Emmmer S via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG65f6a8c23b53: [LLDB] Fix: make m_target_arch private 
(authored by Emmmer).

Changed prior to commit:
  https://reviews.llvm.org/D132353?vs=454426&id=454434#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132353

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
@@ -61,9 +61,9 @@
 return nullptr;
   }
 
-  // FIXME make private.
+private:
   lldb_private::ArchSpec m_target_arch;
 };
-}
+} // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
@@ -399,7 +399,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 m_reg_info.num_registers = k_num_registers_i386;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_i386;
@@ -518,7 +518,7 @@
 
 const RegisterSet *RegisterContextPOSIX_x86::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::x86:
   return &g_reg_sets_i386[set];
 case llvm::Triple::x86_64:
Index: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_s390x.cpp
@@ -83,7 +83,7 @@
 : RegisterContext(thread, concrete_frame_idx) {
   m_register_info_up.reset(register_info);
 
-  switch (register_info->m_target_arch.GetMachine()) {
+  switch (register_info->GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::systemz:
 m_reg_info.num_registers = k_num_registers_s390x;
 m_reg_info.num_gpr_registers = k_num_gpr_registers_s390x;
@@ -151,7 +151,7 @@
 
 const RegisterSet *RegisterContextPOSIX_s390x::GetRegisterSet(size_t set) {
   if (IsRegisterSetAvailable(set)) {
-switch (m_register_info_up->m_target_arch.GetMachine()) {
+switch (m_register_info_up->GetTargetArchitecture().GetMachine()) {
 case llvm::Triple::systemz:
   return &g_reg_sets_s390x[set];
 default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextOpenBSD_i386.cpp
@@ -62,7 +62,7 @@
 size_t RegisterContextOpenBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextOpenBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 return g_register_infos_i386;
   default:
Index: lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
@@ -80,7 +80,7 @@
 size_t RegisterContextNetBSD_i386::GetGPRSize() const { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextNetBSD_i386::GetRegisterInfo() const {
-  switch (m_target_arch.GetMachine()) {
+  switch (GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 return g_register_infos_i386;
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
+

[Lldb-commits] [lldb] b21de9b - [lldb] Silence a GCC warning about missing returns after a fully covered switch. NFC.

2022-08-22 Thread Martin Storsjö via lldb-commits 

Author: Martin Storsjö
Date: 2022-08-22T14:53:29+03:00
New Revision: b21de9b38f4b5f2ae6d49f973b683f118b9d58cb

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

LOG: [lldb] Silence a GCC warning about missing returns after a fully covered 
switch. NFC.

Added: 


Modified: 
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Removed: 




diff  --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h 
b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index 39f7c429106bc..65fba83eb14f7 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -33,6 +33,7 @@ class EmulateInstructionRISCV : public EmulateInstruction {
 case eInstructionTypeAll:
   return false;
 }
+llvm_unreachable("Fully covered switch above!");
   }
 
   static bool SupportsThisArch(const ArchSpec &arch);



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

[Lldb-commits] [lldb] 6f19f98 - [lldb][Test] Replace expect() with expect_expr() in TestNamespaceLookup.py

2022-08-22 Thread Michael Buch via lldb-commits 

Author: Michael Buch
Date: 2022-08-22T13:22:19+01:00
New Revision: 6f19f98710f897ecaf148f42da6eec9d14631449

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

LOG: [lldb][Test] Replace expect() with expect_expr() in TestNamespaceLookup.py

This will be useful in preparation for some reshuffling
of assertions in this file since we won't have to
adjust the persitent variable names during the process.

sed commands:
```
s/expect("expr -- /expect_expr("/g
s/startstr="(int) [$0-9]* = /result_type="int", result_value="/g
```

**Testing**

* API tests still pass

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

Added: 


Modified: 
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py 
b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
index 3a27278beae84..cf33919855d8c 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -82,60 +82,60 @@ def test_scope_lookup_with_run_command(self):
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
 # Evaluate func() - should call ::func()
-self.expect("expr -- func()", startstr="(int) $0 = 1")
+self.expect_expr("func()", result_type="int", result_value="1")
 # Evaluate A::B::func() - should call A::B::func()
-self.expect("expr -- A::B::func()", startstr="(int) $1 = 4")
+self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
-self.expect("expr -- func(10)", startstr="(int) $2 = 11")
+self.expect_expr("func(10)", result_type="int", result_value="11")
 # Evaluate ::func() - should call A::func()
-self.expect("expr -- ::func()", startstr="(int) $3 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
-self.expect("expr -- A::foo()", startstr="(int) $4 = 42")
+self.expect_expr("A::foo()", result_type="int", result_value="42")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
-self.expect("expr -- func(10)", startstr="(int) $5 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $6 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 # Evaluate func() - should call A::func()
-self.expect("expr -- func()", startstr="(int) $7 = 3")
+self.expect_expr("func()", result_type="int", result_value="3")
 
 # Continue to BP_nested_ns_scope at nested ns scope
 self.runToBkpt("continue")
 # Evaluate func() - should call A::B::func()
-self.expect("expr -- func()", startstr="(int) $8 = 4")
+self.expect_expr("func()", result_type="int", result_value="4")
 # Evaluate A::func() - should call A::func()
-self.expect("expr -- A::func()", startstr="(int) $9 = 3")
+self.expect_expr("A::func()", result_type="int", result_value="3")
 
 # Evaluate func(10) - should call A::func(10)
 # NOTE: Under the rules of C++, this test would normally get an error
 # because A::B::func() hides A::func(), but lldb intentionally
 # disobeys these rules so that the intended overload can be found
 # by only removing duplicates if they have the same type.
-self.expect("expr -- func(10)", startstr="(int) $10 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 
 # Continue to BP_nested_ns_scope_after_using at nested ns scope after
 # using declaration
 self.runToBkpt("continue")
 # Evaluate A::func(10) - should call A::func(int)
-self.expect("expr -- A::func(10)", startstr="(int) $11 = 13")
+self.expect_expr("A::func(10)", result_type="int", result_value="13")
 
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $12 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $13 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # decla

[Lldb-commits] [PATCH] D132271: [lldb][Test] Replace expect() with expect_expr() in TestNamespaceLookup.py

2022-08-22 Thread Michael Buch via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f19f98710f8: [lldb][Test] Replace expect() with 
expect_expr() in TestNamespaceLookup.py (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132271

Files:
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -82,60 +82,60 @@
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
 # Evaluate func() - should call ::func()
-self.expect("expr -- func()", startstr="(int) $0 = 1")
+self.expect_expr("func()", result_type="int", result_value="1")
 # Evaluate A::B::func() - should call A::B::func()
-self.expect("expr -- A::B::func()", startstr="(int) $1 = 4")
+self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
-self.expect("expr -- func(10)", startstr="(int) $2 = 11")
+self.expect_expr("func(10)", result_type="int", result_value="11")
 # Evaluate ::func() - should call A::func()
-self.expect("expr -- ::func()", startstr="(int) $3 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
-self.expect("expr -- A::foo()", startstr="(int) $4 = 42")
+self.expect_expr("A::foo()", result_type="int", result_value="42")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
-self.expect("expr -- func(10)", startstr="(int) $5 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $6 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 # Evaluate func() - should call A::func()
-self.expect("expr -- func()", startstr="(int) $7 = 3")
+self.expect_expr("func()", result_type="int", result_value="3")
 
 # Continue to BP_nested_ns_scope at nested ns scope
 self.runToBkpt("continue")
 # Evaluate func() - should call A::B::func()
-self.expect("expr -- func()", startstr="(int) $8 = 4")
+self.expect_expr("func()", result_type="int", result_value="4")
 # Evaluate A::func() - should call A::func()
-self.expect("expr -- A::func()", startstr="(int) $9 = 3")
+self.expect_expr("A::func()", result_type="int", result_value="3")
 
 # Evaluate func(10) - should call A::func(10)
 # NOTE: Under the rules of C++, this test would normally get an error
 # because A::B::func() hides A::func(), but lldb intentionally
 # disobeys these rules so that the intended overload can be found
 # by only removing duplicates if they have the same type.
-self.expect("expr -- func(10)", startstr="(int) $10 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 
 # Continue to BP_nested_ns_scope_after_using at nested ns scope after
 # using declaration
 self.runToBkpt("continue")
 # Evaluate A::func(10) - should call A::func(int)
-self.expect("expr -- A::func(10)", startstr="(int) $11 = 13")
+self.expect_expr("A::func(10)", result_type="int", result_value="13")
 
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $12 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $13 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $14 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $15 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 @expectedFailure("lldb scope lookup of functions bugs")
 def test_function_scope_lookup_with_run_command(self):
@@ -161,18 +161,18 @@
 # Evaluate foo() - should cal

[Lldb-commits] [lldb] 09f608f - [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

2022-08-22 Thread Michael Buch via lldb-commits 

Author: Michael Buch
Date: 2022-08-22T13:22:20+01:00
New Revision: 09f608fda51ca9dd2d88c2985bad1cfc1e36251e

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

LOG: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

This check was put in place to prevent static functions
from translation units outside the one that the current
expression is evaluated from taking precedence over functions
in the global namespace. However, this is really a different
bug. LLDB lumps functions from all CUs into a single AST and
ends up picking the file-static even when C++ context rules
wouldn't allow that to happen.

This patch removes the check so we apply the AsmLabel to all
FunctionDecls we create from DWARF if we have a linkage name
available. This makes the code-path easier to reason about and
allows calling static functions in contexts where we previously
would've chosen the wrong function.

We also flip the XFAILs in the API test to reflect what effect
this change has.

**Testing**

* Fixed API tests and added XFAIL

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index fda68f289a397..9fe9b2e25a2f8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1258,7 +1258,7 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE &die,
   // example is generating calls to ABI-tagged template functions.
   // This is done separately for member functions in
   // AddMethodToCXXRecordType.
-  if (attrs.mangled_name && attrs.storage == clang::SC_Extern)
+  if (attrs.mangled_name)
 function_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(
 m_ast.getASTContext(), attrs.mangled_name, /*literal=*/false));
 

diff  --git a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py 
b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
index cf33919855d8c..789b33c87256e 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -36,6 +36,55 @@ def runToBkpt(self, command):
 substrs=['stopped',
  'stop reason = breakpoint'])
 
+@skipIfWindows # This is flakey on Windows: llvm.org/pr38373
+@expectedFailure("CU-local objects incorrectly scoped")
+def test_scope_lookup_with_run_command_globals(self):
+"""Test scope lookup of functions in lldb."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self,
+self.line_break_global_scope,
+lldb.SBFileSpec("ns.cpp"))
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"ns3.cpp",
+self.line_break_before_using_directive,
+num_expected_locations=1,
+loc_exact=False)
+
+# Run to BP_global_scope at file scope
+self.runToBkpt("run")
+
+# FIXME: LLDB does not correctly scope CU-local objects.
+# LLDB currently lumps functions from all files into
+# a single AST and depending on the order with which
+# functions are considered, LLDB can incorrectly call
+# the static local ns.cpp::func() instead of the expected
+# ::func()
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", expect_type="int", expect_value="1")
+
+# Evaluate ::func() - should call A::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_before_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", result_type="int", result_value="1")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_after_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_with_run_command(self):
 """Test scope lookup of functions in lldb."""
@@ -66,6 +115,12 @@ def test_scope_lookup_with_run_command(self):
 self.line_break_nested_ns_scope_after_using,
 num_exp

[Lldb-commits] [PATCH] D132231: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

2022-08-22 Thread Michael Buch via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG09f608fda51c: [lldb][ClangExpression] Remove storage-class 
check when creating AsmLabel (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132231

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -36,6 +36,55 @@
 substrs=['stopped',
  'stop reason = breakpoint'])
 
+@skipIfWindows # This is flakey on Windows: llvm.org/pr38373
+@expectedFailure("CU-local objects incorrectly scoped")
+def test_scope_lookup_with_run_command_globals(self):
+"""Test scope lookup of functions in lldb."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self,
+self.line_break_global_scope,
+lldb.SBFileSpec("ns.cpp"))
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"ns3.cpp",
+self.line_break_before_using_directive,
+num_expected_locations=1,
+loc_exact=False)
+
+# Run to BP_global_scope at file scope
+self.runToBkpt("run")
+
+# FIXME: LLDB does not correctly scope CU-local objects.
+# LLDB currently lumps functions from all files into
+# a single AST and depending on the order with which
+# functions are considered, LLDB can incorrectly call
+# the static local ns.cpp::func() instead of the expected
+# ::func()
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", expect_type="int", expect_value="1")
+
+# Evaluate ::func() - should call A::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_before_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate func() - should call ::func()
+self.expect_expr("func()", result_type="int", result_value="1")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
+# Continue to BP_after_using_directive at file scope
+self.runToBkpt("continue")
+
+# Evaluate ::func() - should call ::func()
+self.expect_expr("::func()", result_type="int", result_value="1")
+
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_with_run_command(self):
 """Test scope lookup of functions in lldb."""
@@ -66,6 +115,12 @@
 self.line_break_nested_ns_scope_after_using,
 num_expected_locations=1,
 loc_exact=False)
+lldbutil.run_break_set_by_file_and_line(
+self,
+"ns2.cpp",
+self.line_break_file_scope,
+num_expected_locations=1,
+loc_exact=False)
 lldbutil.run_break_set_by_file_and_line(
 self,
 "ns3.cpp",
@@ -81,17 +136,18 @@
 
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
-# Evaluate func() - should call ::func()
-self.expect_expr("func()", result_type="int", result_value="1")
+
 # Evaluate A::B::func() - should call A::B::func()
 self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
 self.expect_expr("func(10)", result_type="int", result_value="11")
-# Evaluate ::func() - should call A::func()
-self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
 self.expect_expr("A::foo()", result_type="int", result_value="42")
 
+# Continue to BP_file_scope at file scope
+self.runToBkpt("continue")
+self.expect_expr("func()", result_type="int", result_value="2")
+
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
@@ -124,16 +180,12 @@
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
-# Evaluate ::func() - should call ::func()
-self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
 self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # declaration
 self.runToBkpt("continue")
-# Evaluate

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione created this revision.
kastiglione added reviewers: augusto2112, JDevlieghere, mib, aprantl.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Remove the test override of `target.prefer-dynamic-value`.

Previously, the lldb default was `no-dynamic-values`. In rG9aa7e8e9ffbe 
 (in
2015), the default was changed to `no-run-target`, but at that time the tests
were changed to be run with `no-dynamic-value`. I don't the reasons for not
changing the tests, perhaps to avoid determining which tests to change and how.

Because `no-run-target` is the lldb default, I think it makes sense to make it
the test default too. It puts the test config closer to what's used in
practice.

This change removes the `target.prefer-dynamic-value` override, and for those
tests that failed, they have been updated to explicitly use
`no-dynamic-values`. Future changes could update these tests to use dynamic
values too, or they can be left as is to exercise non-dynamic typing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132382

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
  
lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCExpr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSData.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSNumber.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/test/API/functionalities/data-formatter/nsarraysynth/TestNSArraySynthetic.py
  
lldb/test/API/functionalities/data-formatter/nsdictionarysynth/TestNSDictionarySynthetic.py
  lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
  lldb/test/API/lang/objc/blocks/TestObjCIvarsInBlocks.py
  lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
  lldb/test/API/lang/objc/foundation/TestObjCMethods.py
  lldb/test/API/lang/objc/foundation/TestObjCMethodsString.py
  lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
  lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
  lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
  lldb/test/API/sanity/TestSettingSkipping.py

Index: lldb/test/API/sanity/TestSettingSkipping.py
===
--- lldb/test/API/sanity/TestSettingSkipping.py
+++ lldb/test/API/sanity/TestSettingSkipping.py
@@ -12,12 +12,12 @@
 
   NO_DEBUG_INFO_TESTCASE = True
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @skipIf(py_version=('>=', (3, 0)))
   def testSkip(self):
 """This setting is on by default"""
 self.assertTrue(False, "This test should not run!")
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  @skipIf(py_version=('<', (3, 0)))
   def testNoMatch(self):
 self.assertTrue(True, "This test should run!")
 
@@ -25,11 +25,11 @@
   def testNotExisting(self):
 self.assertTrue(True, "This test should run!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @expectedFailureAll(py_version=('>=', (3, 0)))
   def testXFAIL(self):
 self.assertTrue(False, "This test should run and fail!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'run-target'))
+  @expectedFailureAll(py_version=('<', (3, 0)))
   def testNotXFAIL(self):
 self.assertTrue(True, "This test should run and succeed!")
 
Index: lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
===
--- lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
+++ lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
@@ -28,6 +28,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",
@@ -53,6 +55,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/test/API/sanity/TestSettingSkipping.py:15-32
+  @skipIf(py_version=('>=', (3, 0)))
   def testSkip(self):
 """This setting is on by default"""
 self.assertTrue(False, "This test should not run!")
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  @skipIf(py_version=('<', (3, 0)))
   def testNoMatch(self):

these tests had nothing to do with `prefer-dynamic-value`, they picked 
something to decorate on. I picked something else to decorate on. I couldn't 
pick a setting now that `configuration.settings` is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

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

[Lldb-commits] [PATCH] D132395: [lldb] [gdb-remote] Use Communication::WriteAll() over Write()

2022-08-22 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, jingham.
Herald added subscribers: kristof.beyls, arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Replace the uses of Communication::Write() with WriteAll() to avoid
partial writes.  None of the call sites actually accounted for that
possibility and even if it is unlikely to actually happen, there doesn't
seem to be any real harm from using WriteAll() instead.

Ideally, we'd remove Write() from the public API.  However, that would
change the API of SBCommunication.  The alternative would be to alias it
to WriteAll().

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D132395

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp


Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
@@ -39,7 +39,7 @@
 
   bool Write(llvm::StringRef packet) {
 ConnectionStatus status;
-return server.Write(packet.data(), packet.size(), status, nullptr) ==
+return server.WriteAll(packet.data(), packet.size(), status, nullptr) ==
packet.size();
   }
 };
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
@@ -2880,7 +2880,7 @@
   Status &error) {
   if (m_stdio_communication.IsConnected()) {
 ConnectionStatus status;
-m_stdio_communication.Write(src, src_len, status, nullptr);
+m_stdio_communication.WriteAll(src, src_len, status, nullptr);
   } else if (m_stdin_forward) {
 m_gdb_comm.SendStdinNotification(src, src_len);
   }
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
@@ -2441,7 +2441,7 @@
 // remote host
 ConnectionStatus status;
 Status error;
-m_stdio_communication.Write(tmp, read, status, &error);
+m_stdio_communication.WriteAll(tmp, read, status, &error);
 if (error.Fail()) {
   return SendErrorResponse(0x15);
 }


Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
@@ -39,7 +39,7 @@
 
   bool Write(llvm::StringRef packet) {
 ConnectionStatus status;
-return server.Write(packet.data(), packet.size(), status, nullptr) ==
+return server.WriteAll(packet.data(), packet.size(), status, nullptr) ==
packet.size();
   }
 };
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
@@ -2880,7 +2880,7 @@
   Status &error) {
   if (m_stdio_communication.IsConnected()) {
 ConnectionStatus status;
-m_stdio_communication.Write(src, src_len, status, nullptr);
+m_stdio_communication.WriteAll(src, src_len, status, nullptr);
   } else if (m_stdin_forward) {
 m_gdb_comm.SendStdinNotification(src, src_len);
   }
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
@@ -2441,7 +2441,7 @@
 // remote host
 ConnectionStatus status;
 Status error;
-m_stdio_communication.Write(tmp, read, status, &error);
+m_stdio_communication.WriteAll(tmp, read, status, &error);
 if (error.Fail()) {
   return SendErrorResponse(0x15);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132397: [LLDB] Clean up after command fails

2022-08-22 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu created this revision.
zequanwu added a reviewer: JDevlieghere.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

`CommandObject::CheckRequirements()` requires m_exe_ctx being cleaned up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132397

Files:
  lldb/source/Interpreter/CommandObject.cpp


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -741,6 +741,7 @@
 if (cmd_args.GetArgumentCount() != 0 && m_arguments.empty()) {
   result.AppendErrorWithFormatv("'{0}' doesn't take any arguments.",
 GetCommandName());
+  Cleanup();
   return false;
 }
 handled = DoExecute(cmd_args, result);


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -741,6 +741,7 @@
 if (cmd_args.GetArgumentCount() != 0 && m_arguments.empty()) {
   result.AppendErrorWithFormatv("'{0}' doesn't take any arguments.",
 GetCommandName());
+  Cleanup();
   return false;
 }
 handled = DoExecute(cmd_args, result);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132307: [lldb] Switch RegularExpression from llvm::Regex to std::regex

2022-08-22 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Utility/CMakeLists.txt:29
+  PROPERTIES COMPILE_OPTIONS
+  "-fcxx-exceptions"
+)

labath wrote:
> kastiglione wrote:
> > kastiglione wrote:
> > > JDevlieghere wrote:
> > > > kastiglione wrote:
> > > > > the `std::regex` constructor throws `std::regex_error` if the pattern 
> > > > > is invalid. For this reason, exceptions are enabled for this one file.
> > > > What happens when exceptions are disabled? What does it mean to have 
> > > > this enabled for a single file? I don't know if it's part of the LLVM 
> > > > developer guide, but LLVM is supposed to build without RTTI and without 
> > > > exceptions. 
> > > > What happens when exceptions are disabled?
> > > 
> > > This cmake config enables exceptions for this one file, independent of 
> > > `LLVM_ENABLE_EH`. No other source files will be allowed to catch or throw 
> > > exceptions.
> > > 
> > > > What does it mean to have this enabled for a single file?
> > > 
> > > It means this file can compile with a try/catch, and that inside this 
> > > file, exceptions can be caught.
> > > 
> > > > I don't know if it's part of the LLVM developer guide, but LLVM is 
> > > > supposed to build without RTTI and without exceptions.
> > > 
> > > llvm has `LLVM_ENABLE_EH` which allows llvm to be built with exceptions 
> > > support enabled. Similarly, `LLVM_ENABLE_RTTI` allows RTTI to be enabled. 
> > > It seems that both are disabled as a default, but not as a hard 
> > > requirement.
> > > 
> > > I wondered if enabling RTTI would needed for exceptions, but at least for 
> > > this code, the answer is no. The `catch (const std::regex_error &e)` 
> > > block is exercised by `TestBreakpointRegexError.py`, so we know this code 
> > > can and does catch an exception of that type, and accesses the error's 
> > > member functions.
> > > 
> > > 
> > What makes me believe this use of exceptions is ok is that the API boundary 
> > continues to be exception free, callers continue to be exception disabled. 
> > Only the internal implementation, knows about exceptions.
> That is definitely not ok. ODR madness awaits therein. The standard library 
> is full of functions that effectively do
> ```
> #ifdef EXCEPTIONS
>   something();
> #else
>   something_else();
> #endif
> ```
> Compiling just one file with exceptions enabled can cause two different 
> versions of those functions to appear. It is sufficient that this file 
> instantiates one function whose behavior is dependent on exceptions being 
> available. When linking, the linker has to choose one of the two versions, 
> and there's no telling which one will it use. This means that exceptions can 
> creep in into the supposedly exception-free code, or vice-versa.
> 
> The only way this would be safe is if this code was in a shared library, and 
> we took care to restrict the visibility of all symbols except the exported 
> api of that library. And I don't think that's worth it.
@labath thanks for pointing that out. I had incomplete understanding and 
thought it could still work, but I see now that it could be an issue.

> The only way this would be safe is if this code was in a shared library, and 
> we took care to restrict the visibility of all symbols except the exported 
> api of that library. And I don't think that's worth it.

what are your concerns with this approach?

btw I spoke to Louis Dionne of libc++ and he said that it could be possible to 
include whether `-fno-exceptions` is used in the libc++ ABI tag, which would 
allow mixing files built with and without exceptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132307

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

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added a comment.

> were changed to be run with no-dynamic-value. I don't the reasons for not
> changing the tests, perhaps to avoid determining which tests to change and 
> how.

I don't ** know ** the reasons ... ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

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

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

I've had this issue in the past in a test where the variable value would not 
match what I'd get on an interactive debug session, until I added that flag.

I think this makes sense to have consistency between the interactive session 
default and the test suite. LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

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

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 accepted this revision.
augusto2112 added a comment.

I think this is a good change. One related thing that we could look into fixing 
is that currently both run-target and no-run-target do the **exact** same 
thing. Not sure if we could remove one of them since this could break user's 
settings, but we could at least document it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

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

[Lldb-commits] [PATCH] D132307: [lldb] Switch RegularExpression from llvm::Regex to std::regex

2022-08-22 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added a comment.

This would be pretty neat, if we're able to make it work without creating 
side-effects to other component.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132307

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

[Lldb-commits] [PATCH] D129521: Add the ability to run expressions that call fork() or vfork().

2022-08-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Ping. I would love to get this in if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129521

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

[Lldb-commits] [PATCH] D131705: Don't create sections for SHN_ABS symbols in ELF files.

2022-08-22 Thread Greg Clayton via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0697d7c3fb5: Don't create sections for SHN_ABS symbols 
in ELF files. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131705

Files:
  lldb/bindings/interface/SBSymbol.i
  lldb/include/lldb/API/SBSymbol.h
  lldb/include/lldb/Symbol/Symbol.h
  lldb/source/API/SBSymbol.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/test/API/python_api/absolute_symbol/TestAbsoluteSymbol.py
  lldb/test/API/python_api/absolute_symbol/absolute.yaml
  lldb/test/Shell/SymbolFile/absolute-symbol.test

Index: lldb/test/Shell/SymbolFile/absolute-symbol.test
===
--- lldb/test/Shell/SymbolFile/absolute-symbol.test
+++ lldb/test/Shell/SymbolFile/absolute-symbol.test
@@ -1,8 +1,7 @@
 # RUN: yaml2obj %s -o %t.o
 # RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck %s
 # CHECK: 1 symbols match 'absolute_symbol'
-# CHECK:   Address: 0x12345678 (0x12345678)
-# CHECK:   Summary: 0x12345678
+# CHECK:   Value: 0x12345678
 # Created from:
 #   .globl absolute_symbol
 #   absolute_symbol = 0x12345678
@@ -92,4 +91,3 @@
 - ltmp0
 - ''
 ...
-
Index: lldb/test/API/python_api/absolute_symbol/absolute.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/absolute_symbol/absolute.yaml
@@ -0,0 +1,36 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x8037C000
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x1000
+AddressAlign:0x4
+Content: "c3c3c3c3"
+ProgramHeaders:
+  - Type: PT_LOAD
+Flags: [ PF_X, PF_R ]
+VAddr: 0x1000
+PAddr: 0x1000
+Align: 0x4
+FirstSec: .text
+LastSec:  .text
+Symbols:
+  - Name:main
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x1000
+Size:0x4
+  - Name:absolute
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x8000
+Size:0x9
+
Index: lldb/test/API/python_api/absolute_symbol/TestAbsoluteSymbol.py
===
--- /dev/null
+++ lldb/test/API/python_api/absolute_symbol/TestAbsoluteSymbol.py
@@ -0,0 +1,80 @@
+"""
+Test absolute symbols in ELF files to make sure they don't create sections and
+to verify that symbol values and size can still be accessed via SBSymbol APIs.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import os
+
+class TestAbsoluteSymbol(TestBase):
+
+@no_debug_info_test
+def test_absolute_symbol(self):
+'''
+Load an ELF file that contains two symbols:
+- "absolute" which is a symbol with the section SHN_ABS
+- "main" which is a code symbol in .text
+
+Index   st_namest_value   st_sizest_info st_other st_shndx Name
+=== -- -- -- ---   ===
+[0] 0x 0x 0x 0x00 (STB_LOCAL  STT_NOTYPE   ) 0x000
+[1] 0x0001 0x1000 0x0004 0x12 (STB_GLOBAL STT_FUNC ) 0x001 main
+[2] 0x0006 0x8000 0x0009 0x10 (STB_GLOBAL STT_NOTYPE   ) 0x00  SHN_ABS absolute
+
+We used to create sections for symbols whose section ID was SHN_ABS
+and this caused problems as the new sections could interfere with
+with address resolution. Absolute symbols' values are not addresses
+and should not be treated this way.
+
+New APIs were added to SBSymbol to allow access to the raw integer
+value and size of symbols so symbols whose value was not an address
+could be accessed. Prior to this commit, you could only call:
+
+SBAddress SBSymbol::GetStartAddress()
+SBAddress SBSymbol::GetEndAddress()
+
+If the symbol's value was not an address, you couldn't access the
+raw value because the above accessors would return invalid SBAddress
+objects if the value wasn't an address. New APIs were added for this:
+
+

[Lldb-commits] [lldb] f0697d7 - Don't create sections for SHN_ABS symbols in ELF files.

2022-08-22 Thread Greg Clayton via lldb-commits 

Author: Greg Clayton
Date: 2022-08-22T14:46:27-07:00
New Revision: f0697d7c3fb5296cfec1718206aceb77b7ca9ab8

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

LOG: Don't create sections for SHN_ABS symbols in ELF files.

Symbols that have the section index of SHN_ABS were previously creating extra 
top level sections that contained the value of the symbol as if the symbol's 
value was an address. As far as I can tell, these symbol's values are not 
addresses, even if they do have a size. To make matters worse, adding these 
extra sections can stop address lookups from succeeding if the symbol's value + 
size overlaps with an existing section as these sections get mapped into memory 
when the image is loaded by the dynamic loader. This can cause stack frames to 
appear empty as the address lookup fails completely.

This patch:
- doesn't create a section for any SHN_ABS symbols
- makes symbols that are absolute have values that are not addresses
- add accessors to SBSymbol to get the value and size of a symbol as raw 
integers. Prevoiusly there was no way to access a symbol's value from a 
SBSymbol because the only accessors were:

  SBAddress SBSymbol::GetStartAddress();
  SBAddress SBSymbol::GetEndAddress();

  and these accessors would return an invalid SBAddress if the symbol's value 
wasn't an address
- Adds a test to ensure no ".absolute." sections are created
- Adds a test to test the new SBSymbol APIs

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

Added: 
lldb/test/API/python_api/absolute_symbol/TestAbsoluteSymbol.py
lldb/test/API/python_api/absolute_symbol/absolute.yaml

Modified: 
lldb/bindings/interface/SBSymbol.i
lldb/include/lldb/API/SBSymbol.h
lldb/include/lldb/Symbol/Symbol.h
lldb/source/API/SBSymbol.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Symbol/Symbol.cpp
lldb/test/Shell/SymbolFile/absolute-symbol.test

Removed: 




diff  --git a/lldb/bindings/interface/SBSymbol.i 
b/lldb/bindings/interface/SBSymbol.i
index fa0b3e4e1378d..f5efee75e249d 100644
--- a/lldb/bindings/interface/SBSymbol.i
+++ b/lldb/bindings/interface/SBSymbol.i
@@ -49,6 +49,10 @@ public:
 SBAddress
 GetEndAddress ();
 
+uint64_t GetValue();
+
+uint64_t GetSize();
+
 uint32_t
 GetPrologueByteSize ();
 

diff  --git a/lldb/include/lldb/API/SBSymbol.h 
b/lldb/include/lldb/API/SBSymbol.h
index 5935ccfed0245..94521881f82f9 100644
--- a/lldb/include/lldb/API/SBSymbol.h
+++ b/lldb/include/lldb/API/SBSymbol.h
@@ -41,10 +41,46 @@ class LLDB_API SBSymbol {
   lldb::SBInstructionList GetInstructions(lldb::SBTarget target,
   const char *flavor_string);
 
+  /// Get the start address of this symbol
+  ///
+  /// \returns
+  ///   If the symbol's value is not an address, an invalid SBAddress object
+  ///   will be returned. If the symbol's value is an address, a valid 
SBAddress
+  ///   object will be returned.
   SBAddress GetStartAddress();
 
+  /// Get the end address of this symbol
+  ///
+  /// \returns
+  ///   If the symbol's value is not an address, an invalid SBAddress object
+  ///   will be returned. If the symbol's value is an address, a valid 
SBAddress
+  ///   object will be returned.
   SBAddress GetEndAddress();
 
+  /// Get the raw value of a symbol.
+  ///
+  /// This accessor allows direct access to the symbol's value from the symbol
+  /// table regardless of what the value is. The value can be a file address or
+  /// it can be an integer value that depends on what the symbol's type is. 
Some
+  /// symbol values are not addresses, but absolute values or integer values
+  /// that can be mean 
diff erent things. The GetStartAddress() accessor will
+  /// only return a valid SBAddress if the symbol's value is an address, so 
this
+  /// accessor provides a way to access the symbol's value when the value is
+  /// not an address.
+  ///
+  /// \returns
+  ///   Returns the raw integer value of a symbol from the symbol table.
+  uint64_t GetValue();
+
+  /// Get the size of the symbol.
+  ///
+  /// This accessor allows direct access to the symbol's size from the symbol
+  /// table regardless of what the value is (address or integer value).
+  ///
+  /// \returns
+  ///   Returns the size of a symbol from the symbol table.
+  uint64_t GetSize();
+
   uint32_t GetPrologueByteSize();
 
   SymbolType GetType();

diff  --git a/lldb/include/lldb/Symbol/Symbol.h 
b/lldb/include/lldb/Symbol/Symbol.h
index 23a68090ff4d6..164fafd437dcb 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -85,6 +85,16 @@ class Symbol : public SymbolContextScope {
   return Address();
   }
 
+  /// Get

[Lldb-commits] [lldb] ae376fe - Modify all register values whose byte size matches the address size to be formatter as eFormatAddressInfo.

2022-08-22 Thread Greg Clayton via lldb-commits 

Author: Greg Clayton
Date: 2022-08-22T14:48:16-07:00
New Revision: ae376feb0b1922294884257811b9659eed83e1e1

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

LOG: Modify all register values whose byte size matches the address size to be 
formatter as eFormatAddressInfo.

This allows users to see similar output to what the "register read" command 
emits in LLDB's command line.

Added a test to verify that the PC has the correct value with contains a 
pointer followed by the module + function name and the source line info. 
Something like:

0x00010a64 a.out`main + 132 at main.cpp:17:11

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 8a871732f7a61..0996d8696cd25 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -447,6 +447,10 @@ def get_local_variables(self, frameIndex=0, threadId=None):
 return self.get_scope_variables('Locals', frameIndex=frameIndex,
 threadId=threadId)
 
+def get_registers(self, frameIndex=0, threadId=None):
+return self.get_scope_variables('Registers', frameIndex=frameIndex,
+threadId=threadId)
+
 def get_local_variable(self, name, frameIndex=0, threadId=None):
 locals = self.get_local_variables(frameIndex=frameIndex,
   threadId=threadId)

diff  --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py 
b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index b4ed393290c3d..e4cb103010fde 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -459,3 +459,52 @@ def test_indexedVariables(self):
 "pt": {"missing": ["indexedVariables"]},
 }
 self.verify_variables(verify_locals, locals)
+
+@skipIfWindows
+@skipIfRemote
+def test_registers(self):
+'''
+Test that registers whose byte size is the size of a pointer on
+the current system get formatted as lldb::eFormatAddressInfo. This
+will show the pointer value followed by a description of the 
address
+itself. To test this we attempt to find the PC value in the general
+purpose registers, and since we will be stopped in main.cpp, verify
+that the value for the PC starts with a pointer and is followed by
+a description that contains main.cpp.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+breakpoint1_line = line_number(source, "// breakpoint 1")
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(
+len(breakpoint_ids), len(lines), "expect correct number of 
breakpoints"
+)
+self.continue_to_breakpoints(breakpoint_ids)
+
+
+pc_name = None
+arch = self.getArchitecture()
+if arch == 'x86_64':
+pc_name = 'rip'
+elif arch == 'x86':
+pc_name = 'rip'
+elif arch.startswith('arm'):
+pc_name = 'pc'
+
+if pc_name is None:
+return
+# Verify locals
+reg_sets = self.vscode.get_registers()
+for reg_set in reg_sets:
+if reg_set['name'] == 'General Purpose Registers':
+varRef = reg_set['variablesReference']
+regs = 
self.vscode.request_variables(varRef)['body']['variables']
+for reg in regs:
+if reg['name'] == pc_name:
+value = reg['value']
+self.assertTrue(value.startswith('0x'))
+self.assertTrue('a.out`main + ' in value)
+self.assertTrue('at main.cpp:' in value)

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 6c7118890f2f7..22ff84f42921b 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2933,6 +2933,25 @@ void request_variables(const llvm::json::Object 
&request) {
 int64_t start_id

[Lldb-commits] [PATCH] D129528: Modify all register values whose byte size matches the address size to be formatter as eFormatAddressInfo.

2022-08-22 Thread Greg Clayton via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGae376feb0b19: Modify all register values whose byte size 
matches the address size to be… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129528

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  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
@@ -2933,6 +2933,25 @@
 int64_t start_idx = 0;
 int64_t num_children = 0;
 
+if (variablesReference == VARREF_REGS) {
+  // Change the default format of any pointer sized registers in the first
+  // register set to be the lldb::eFormatAddressInfo so we show the pointer
+  // and resolve what the pointer resolves to. Only change the format if the
+  // format was set to the default format or if it was hex as some registers
+  // have formats set for them.
+  const uint32_t addr_size = g_vsc.target.GetProcess().GetAddressByteSize();
+  lldb::SBValue reg_set = g_vsc.variables.registers.GetValueAtIndex(0);
+  const uint32_t num_regs = reg_set.GetNumChildren();
+  for (uint32_t reg_idx=0; reg_idxGetSize();
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -459,3 +459,52 @@
 "pt": {"missing": ["indexedVariables"]},
 }
 self.verify_variables(verify_locals, locals)
+
+@skipIfWindows
+@skipIfRemote
+def test_registers(self):
+'''
+Test that registers whose byte size is the size of a pointer on
+the current system get formatted as lldb::eFormatAddressInfo. This
+will show the pointer value followed by a description of the address
+itself. To test this we attempt to find the PC value in the general
+purpose registers, and since we will be stopped in main.cpp, verify
+that the value for the PC starts with a pointer and is followed by
+a description that contains main.cpp.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+breakpoint1_line = line_number(source, "// breakpoint 1")
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(
+len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+)
+self.continue_to_breakpoints(breakpoint_ids)
+
+
+pc_name = None
+arch = self.getArchitecture()
+if arch == 'x86_64':
+pc_name = 'rip'
+elif arch == 'x86':
+pc_name = 'rip'
+elif arch.startswith('arm'):
+pc_name = 'pc'
+
+if pc_name is None:
+return
+# Verify locals
+reg_sets = self.vscode.get_registers()
+for reg_set in reg_sets:
+if reg_set['name'] == 'General Purpose Registers':
+varRef = reg_set['variablesReference']
+regs = self.vscode.request_variables(varRef)['body']['variables']
+for reg in regs:
+if reg['name'] == pc_name:
+value = reg['value']
+self.assertTrue(value.startswith('0x'))
+self.assertTrue('a.out`main + ' in value)
+self.assertTrue('at main.cpp:' in value)
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -447,6 +447,10 @@
 return self.get_scope_variables('Locals', frameIndex=frameIndex,
 threadId=threadId)
 
+def get_registers(self, frameIndex=0, threadId=None):
+return self.get_scope_variables('Registers', frameIndex=frameIndex,
+threadId=threadId)
+
 def get_local_variable(self, name, frameIndex=0, threadId=None):
 locals = self.get_local_variables(frameIndex=frameIndex,
   threadId=threadId)
___
lldb-commits maili

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-22 Thread Adrian Vogelsgesang via Phabricator via lldb-commits 
avogelsgesang created this revision.
avogelsgesang added reviewers: ChuanqiXu, aprantl, labath, mib, JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a project: All.
avogelsgesang requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

This patch adds a formatter for `std::coroutine_handle`, both for libc++
and libstdc++. For the type-erased `coroutine_handle<>`, it shows the
`resume` and `destroy` function pointers. For a non-type-erased
`coroutine_handle` it also shows the `promise` value.

With this change, executing the `v t` command on the example from
https://clang.llvm.org/docs/DebuggingCoroutines.html now outputs

  (task) t = {
handle = coro frame = 0xb2a0 {
  resume = 0x5a10 (a.out`coro_task(int, int) at 
llvm-example.cpp:36)
  destroy = 0x6090 (a.out`coro_task(int, int) at 
llvm-example.cpp:36)
}
  }

instead of just

  (task) t = {
handle = {
  __handle_ = 0xb2a0
}
  }

Note, how the symbols for the `resume` and `destroy` function pointer
reveals which coroutine is stored inside the `std::coroutine_handle`.
A follow-up commit will use this fact to infer the coroutine's promise
type and the representation of its internal coroutine state based on
the `resume` and`destroy` pointers.

The same formatter is used for both libc++ and libstdc++. It would
also work for MSVC's standard library, however it is not registered
for MSVC, given that lldb does not provide pretty printers for other
MSVC types, either.

The formatter is in a new added  `Coroutines.{h,cpp}` file because there
does not seem to be an already existing place where we could share
formatters across libc++ and libstdc++. Also, I expect this code to grow
as we improve debugging experience for coroutines further.

**Testing**

- Added API test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132415

Files:
  clang/docs/tools/clang-formatted-files.txt
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -0,0 +1,36 @@
+#include 
+
+// `int_generator` is a stripped down, minimal coroutine generator
+// type.
+struct int_generator {
+  struct promise_type {
+int current_value = -1;
+
+auto get_return_object() {
+  return std::coroutine_handle::from_promise(*this);
+}
+auto initial_suspend() { return std::suspend_always(); }
+auto final_suspend() noexcept { return std::suspend_always(); }
+auto return_void() { return std::suspend_always(); }
+void unhandled_exception() { __builtin_unreachable(); }
+auto yield_value(int v) {
+  current_value = v;
+  return std::suspend_always();
+}
+  };
+
+  std::coroutine_handle hdl;
+
+  int_generator(std::coroutine_handle h) : hdl(h) {}
+  ~int_generator() { hdl.destroy(); }
+};
+
+int_generator my_generator_func() { co_yield 42; }
+
+int main() {
+  int_generator gen = my_generator_func();
+  std::coroutine_handle<> type_erased_hdl = gen.hdl;
+  gen.hdl.resume(); // Break at initial_suspend
+  gen.hdl.resume(); // Break after co_yield
+  // Break at final_suspend
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -0,0 +1,75 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
+
+class TestCoroutineHandle(TestBase):
+def do_test(self, stdlib_type):
+"""Test std::coroutine_handle is displayed correctly."""
+self.build(dictionary={stdlib_type: "1"})
+
+test_generator_func_ptr_re = re.compile(
+r"^\(a.out`my_generator_func\(\) at main.cpp:[0-9]*\)$")
+
+lldbutil.run_to_source_breakpoint(self, '// Break at initial_suspend',
+

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-22 Thread Adrian Vogelsgesang via Phabricator via lldb-commits 
avogelsgesang added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1139
 
+  AddCXXSynthetic( // XXX
+  cpp_category_sp,

I need to remove this. This is a left-over from an earlier implementation sketch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132415

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

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 454636.
kastiglione added a comment.

Fixed description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
  
lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCExpr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSData.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSNumber.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/test/API/functionalities/data-formatter/nsarraysynth/TestNSArraySynthetic.py
  
lldb/test/API/functionalities/data-formatter/nsdictionarysynth/TestNSDictionarySynthetic.py
  lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
  lldb/test/API/lang/objc/blocks/TestObjCIvarsInBlocks.py
  lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
  lldb/test/API/lang/objc/foundation/TestObjCMethods.py
  lldb/test/API/lang/objc/foundation/TestObjCMethodsString.py
  lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
  lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
  lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
  lldb/test/API/sanity/TestSettingSkipping.py

Index: lldb/test/API/sanity/TestSettingSkipping.py
===
--- lldb/test/API/sanity/TestSettingSkipping.py
+++ lldb/test/API/sanity/TestSettingSkipping.py
@@ -12,12 +12,12 @@
 
   NO_DEBUG_INFO_TESTCASE = True
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @skipIf(py_version=('>=', (3, 0)))
   def testSkip(self):
 """This setting is on by default"""
 self.assertTrue(False, "This test should not run!")
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  @skipIf(py_version=('<', (3, 0)))
   def testNoMatch(self):
 self.assertTrue(True, "This test should run!")
 
@@ -25,11 +25,11 @@
   def testNotExisting(self):
 self.assertTrue(True, "This test should run!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @expectedFailureAll(py_version=('>=', (3, 0)))
   def testXFAIL(self):
 self.assertTrue(False, "This test should run and fail!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'run-target'))
+  @expectedFailureAll(py_version=('<', (3, 0)))
   def testNotXFAIL(self):
 self.assertTrue(True, "This test should run and succeed!")
 
Index: lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
===
--- lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
+++ lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
@@ -28,6 +28,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",
@@ -53,6 +55,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",
Index: lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
===
--- lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
+++ lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
@@ -54,6 +54,8 @@
 def test_float_literal(self):
 self.runToBreakpoint()
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 self.expect("expr -- @123.45", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["NSNumber", "123.45"])
 
@@ -62,6 +64,8 @@
 def test_expressions_in_literals(self):
 self.runToBreakpoint()
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 self.expect(
 "expr --object-description -- @( 1 + 3 )",
 VARIABLES_DISPLAYED_CORRECTLY,
Index: lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
===

[Lldb-commits] [PATCH] D132382: [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Dave Lee via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc21dfa9e4931: [lldb] Remove prefer-dynamic-value test 
override (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132382

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
  
lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCExpr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSData.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSNumber.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/test/API/functionalities/data-formatter/nsarraysynth/TestNSArraySynthetic.py
  
lldb/test/API/functionalities/data-formatter/nsdictionarysynth/TestNSDictionarySynthetic.py
  lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
  lldb/test/API/lang/objc/blocks/TestObjCIvarsInBlocks.py
  lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
  lldb/test/API/lang/objc/foundation/TestObjCMethods.py
  lldb/test/API/lang/objc/foundation/TestObjCMethodsString.py
  lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
  lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
  lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
  lldb/test/API/sanity/TestSettingSkipping.py

Index: lldb/test/API/sanity/TestSettingSkipping.py
===
--- lldb/test/API/sanity/TestSettingSkipping.py
+++ lldb/test/API/sanity/TestSettingSkipping.py
@@ -12,12 +12,12 @@
 
   NO_DEBUG_INFO_TESTCASE = True
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @skipIf(py_version=('>=', (3, 0)))
   def testSkip(self):
 """This setting is on by default"""
 self.assertTrue(False, "This test should not run!")
 
-  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  @skipIf(py_version=('<', (3, 0)))
   def testNoMatch(self):
 self.assertTrue(True, "This test should run!")
 
@@ -25,11 +25,11 @@
   def testNotExisting(self):
 self.assertTrue(True, "This test should run!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  @expectedFailureAll(py_version=('>=', (3, 0)))
   def testXFAIL(self):
 self.assertTrue(False, "This test should run and fail!")
 
-  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'run-target'))
+  @expectedFailureAll(py_version=('<', (3, 0)))
   def testNotXFAIL(self):
 self.assertTrue(True, "This test should run and succeed!")
 
Index: lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
===
--- lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
+++ lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
@@ -28,6 +28,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",
@@ -53,6 +55,8 @@
 # Run at stop at main
 lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 # This should display correctly.
 self.expect(
 "frame variable foo->_bar->_hidden_ivar",
Index: lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
===
--- lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
+++ lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
@@ -54,6 +54,8 @@
 def test_float_literal(self):
 self.runToBreakpoint()
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 self.expect("expr -- @123.45", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["NSNumber", "123.45"])
 
@@ -62,6 +64,8 @@
 def test_expressions_in_literals(self):
 self.runToBreakpoint()
 
+self.runCmd("settings set target.prefer-dynamic-value no-dynamic-values")
+
 self.expect(
 "expr --object-description -- @

[Lldb-commits] [lldb] c21dfa9 - [lldb] Remove prefer-dynamic-value test override

2022-08-22 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2022-08-22T15:46:03-07:00
New Revision: c21dfa9e4931a0a339813a5d384fc2eb0deee235

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

LOG: [lldb] Remove prefer-dynamic-value test override

Remove the test override of `target.prefer-dynamic-value`.

Previously, the lldb default was `no-dynamic-values`. In rG9aa7e8e9ffbe (in
2015), the default was changed to `no-run-target`, but at that time the tests
were changed to be run with `no-dynamic-value`. I don't know the reasons for
not changing the tests, perhaps to avoid determining which tests to change, and
what about them to change.

Because `no-run-target` is the lldb default, I think it makes sense to make it
the test default too. It puts the test config closer to what's used in
practice.

This change removes the `target.prefer-dynamic-value` override, and for those
tests that failed, they have been updated to explicitly use
`no-dynamic-values`. Future changes could update these tests to use dynamic
values too, or they can be left as is to exercise non-dynamic typing.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py

lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCExpr.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSData.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSNumber.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py

lldb/test/API/functionalities/data-formatter/nsarraysynth/TestNSArraySynthetic.py

lldb/test/API/functionalities/data-formatter/nsdictionarysynth/TestNSDictionarySynthetic.py
lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
lldb/test/API/lang/objc/blocks/TestObjCIvarsInBlocks.py
lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
lldb/test/API/lang/objc/foundation/TestObjCMethods.py
lldb/test/API/lang/objc/foundation/TestObjCMethodsString.py
lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
lldb/test/API/lang/objc/objc-new-syntax/TestObjCNewSyntaxLiteral.py
lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
lldb/test/API/sanity/TestSettingSkipping.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 5133dedbe8524..2a8d125f2ee94 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -51,8 +51,7 @@
 dwarf_version = 0
 
 # Any overridden settings.
-# Always disable default dynamic types for testing purposes.
-settings = [('target.prefer-dynamic-value', 'no-dynamic-values')]
+settings = []
 
 # Path to the FileCheck testing tool. Not optional.
 filecheck = None

diff  --git a/lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py 
b/lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
index a9131360fef6e..9d143ec546c9d 100644
--- a/lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
+++ b/lldb/test/API/commands/expression/po_verbosity/TestPoVerbosity.py
@@ -41,6 +41,8 @@ def cleanup():
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+self.runCmd("settings set target.prefer-dynamic-value 
no-dynamic-values")
+
 self.expect("expr -O -v -- foo",
 substrs=['(id) $', ' = 0x', '1 = 2', '2 = 3;'])
 self.expect("expr -O -vfull -- foo",

diff  --git 
a/lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
 
b/lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
index a40eb120759eb..044c5324d9013 100644
--- 
a/lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
+++ 
b/lldb/test/API/commands/expression/two-files/TestObjCTypeQueryFromOtherCompileUnit.py
@@ -32,6 +32,8 @@ def test(self):
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+self.runCmd("settings set target.prefer-dynamic-value 
no-dynamic-values")
+
 # Now do a NSArry type query from the 'main.m' compile uint.
 self.expect("expression (NSArray*)array_token",
 substrs=['(N

[Lldb-commits] [PATCH] D131437: Don't index the skeleton CU when we have a fission compile unit.

2022-08-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

So I can't get -fsplit-dwarf-inlining to emit anything when I try to cross with 
clang. I add the flag but no extra function info gets emitted in the dwarf in 
the main executable. I tried:

  clang++ -gdwarf-5 -gsplit-dwarf -fsplit-dwarf-inlining -c main.cpp -o main2.o 

I also tried to create an simple a.out program with a DWO_ID of zero. If it 
obj2yaml and then to yaml2obj, something gets messed up in the binary, so not 
sure if ojb2yaml + yaml2obj can handle fission binaries correctly.

Any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131437

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

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-22 Thread Chuanqi Xu via Phabricator via lldb-commits 
ChuanqiXu added a reviewer: dblaikie.
ChuanqiXu added a comment.

LGTM. But I am not familiar with debugger internals. So I'll leave the formal 
acceptance for other reviewers.




Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:138
+}
\ No newline at end of file


Add the new line.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h:62-64
+bool LibcxxUniquePointerSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::unique_ptr<>

Unrelated change? Generally we could do this in a separate NFC patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132415

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

[Lldb-commits] [PATCH] D132433: [lldb] Teach LLDB about filesets (WIP)

2022-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added a subscriber: mgorny.
Herald added a project: All.
JDevlieghere requested review of this revision.

https://reviews.llvm.org/D132433

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectContainer/CMakeLists.txt
  lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/CMakeLists.txt
  
lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
  
lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -941,6 +941,17 @@
   data.SetData(data_sp, data_offset, data_length);
   lldb::offset_t offset = 0;
   uint32_t magic = data.GetU32(&offset);
+
+  offset += 4; // cputype
+  offset += 4; // cpusubtype
+  uint32_t filetype = data.GetU32(&offset);
+
+  // A fileset has a Mach-O header but is not an
+  // individual file and must be handled via an
+  // ObjectContainer plugin.
+  if (filetype == llvm::MachO::MH_FILESET)
+return false;
+
   return MachHeaderSizeFromMagic(magic) != 0;
 }
 
Index: lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.h
===
--- /dev/null
+++ lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.h
@@ -0,0 +1,78 @@
+//===-- ObjectContainerMachOFileset.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_OBJECTCONTAINER_MACH_O_FILESET_OBJECTCONTAINERMADCHOFILESET_H
+#define LLDB_SOURCE_PLUGINS_OBJECTCONTAINER_MACH_O_FILESET_OBJECTCONTAINERMADCHOFILESET_H
+
+#include "lldb/Host/SafeMachO.h"
+#include "lldb/Symbol/ObjectContainer.h"
+#include "lldb/Utility/FileSpec.h"
+
+namespace lldb_private {
+
+class ObjectContainerMachOFileset : public lldb_private::ObjectContainer {
+public:
+  ObjectContainerMachOFileset(const lldb::ModuleSP &module_sp,
+  lldb::DataBufferSP &data_sp,
+  lldb::offset_t data_offset,
+  const lldb_private::FileSpec *file,
+  lldb::offset_t offset, lldb::offset_t length);
+
+  ~ObjectContainerMachOFileset() override;
+
+  static void Initialize();
+  static void Terminate();
+
+  static llvm::StringRef GetPluginNameStatic() { return "mach-o-fileset"; }
+
+  static llvm::StringRef GetPluginDescriptionStatic() {
+return "Mach-O Fileset container reader.";
+  }
+
+  static lldb_private::ObjectContainer *
+  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+ lldb::offset_t data_offset, const lldb_private::FileSpec *file,
+ lldb::offset_t offset, lldb::offset_t length);
+
+  static size_t GetModuleSpecifications(const lldb_private::FileSpec &file,
+lldb::DataBufferSP &data_sp,
+lldb::offset_t data_offset,
+lldb::offset_t file_offset,
+lldb::offset_t length,
+lldb_private::ModuleSpecList &specs);
+
+  static bool MagicBytesMatch(const lldb_private::DataExtractor &data);
+
+  bool ParseHeader() override;
+
+  void Dump(lldb_private::Stream *s) const override;
+
+  size_t GetNumObjects() const override { return m_entries.size(); }
+
+  lldb::ObjectFileSP GetObjectFile(const lldb_private::FileSpec *file) override;
+
+  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+private:
+  struct Entry {
+Entry(uint64_t vmaddr, uint64_t fileoff, std::string id)
+: vmaddr(vmaddr), fileoff(fileoff), id(id) {}
+uint64_t vmaddr;
+uint64_t fileoff;
+std::string id;
+  };
+
+  static bool ParseHeader(lldb_private::DataExtractor &data,
+  std::vector &entries);
+
+  std::vector m_entries;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_OBJECTCONTAINER_MACH_O_FILESET_OBJECTCONTAINERMADCHOFILESET_H
Index: lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
@@ -0,0 +1,234 @@
+//===-- ObjectContainerMachOFileset.cpp --

[Lldb-commits] [PATCH] D131437: Don't index the skeleton CU when we have a fission compile unit.

2022-08-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 454695.
clayborg added a comment.

Added a test case that tests that we can load a .dwo file with a DWO ID of zero.
Was unable to get clang to emit extra stuff in the skeleton compile unit even 
with -fsplit-dwarf-inlining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131437

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/test/API/functionalities/dwo/TestZeroDwoId.py
  lldb/test/API/functionalities/dwo/main.dwo.yaml
  lldb/test/API/functionalities/dwo/main.o.yaml

Index: lldb/test/API/functionalities/dwo/main.o.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/dwo/main.o.yaml
@@ -0,0 +1,212 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_X86_64
+  SectionHeaderStringTable: .strtab
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x10
+Content: 554889E531C05DC30F1F8400554889E54883EC10C745FC897DF8488975F0E84883C4105DC3
+  - Name:.debug_abbrev
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 01110010171B0EB44219B0420EB1420711011206B3421700
+  - Name:.debug_info
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 2C00040008013100
+  - Name:.debug_addr
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: ''
+  - Name:.debug_gnu_pubnames
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 210002003000190030666F6F002900306D61696E00
+  - Name:.debug_gnu_pubtypes
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 2100020030004F0090696E74006200906368617200
+  - Name:.comment
+Type:SHT_PROGBITS
+Flags:   [ SHF_MERGE, SHF_STRINGS ]
+AddressAlign:0x1
+EntSize: 0x1
+Content: 00636C616E672076657273696F6E2031362E302E30202868747470733A2F2F6769746875622E636F6D2F6C6C766D2F6C6C766D2D70726F6A65637420613836613537613033663831623362323561323839313737373235646662333733303164383836382900
+  - Name:.note.GNU-stack
+Type:SHT_PROGBITS
+AddressAlign:0x1
+  - Name:.eh_frame
+Type:SHT_X86_64_UNWIND
+Flags:   [ SHF_ALLOC ]
+AddressAlign:0x8
+Content: 1400017A5200017810011B0C070890011C001C0008410E108602430D06430C0708001C003C0021410E108602430D065C0C070800
+  - Name:.debug_line
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 47000400210101FB0E0D0001010101000101006D61696E2E63707009020105030A4B0500BD050A0A0859050306580206000101
+  - Name:.rela.text
+Type:SHT_RELA
+Flags:   [ SHF_INFO_LINK ]
+Link:.symtab
+AddressAlign:0x8
+Info:.text
+Relocations:
+  - Offset:  0x27
+Symbol:  _Z3foov
+Type:R_X86_64_PLT32
+Addend:  -4
+  - Name:.rela.debug_info
+Type:SHT_RELA
+Flags:   [ SHF_INFO_LINK ]
+Link:.symtab
+AddressAlign:0x8
+Info:.debug_info
+Relocations:
+  - Offset:  0x6
+Symbol:  .debug_abbrev
+Type:R_X86_64_32
+  - Offset:  0xC
+Symbol:  .debug_line
+Type:R_X86_64_32
+  - Offset:  0x10
+Symbol:  .debug_str
+Type:R_X86_64_32
+  - Offset:  0x14
+Symbol:  .debug_str
+Type:R_X86_64_32
+Addend:  58
+  - Offset:  0x20
+Symbol:  .text
+Type:R_X86_64_64
+  - Offset:  0x2C
+Symbol:  .debug_addr
+Type:R_X86_64_32
+  - N