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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for doing this. Just a couple of comments inline.




Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:288
+
+  unwind_plan_sp = std::make_shared(eRegisterKindGeneric);
+  unwind_plan_sp->AppendRow(row);

I have a feeling that this eRegisterKind enum should correspond with the 
register numbers used in the unwind plan. It'd be great if it did, as then you 
could replace arm64_dwarf::{pc,sp} with LLDB_REGNUM_GENERIC_{PC,SP}, and avoid 
including the arm64 header.  Though if it does work as I remember, then I guess 
the question is how does this work in the first place.



Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:303-305
+  if ((machine == llvm::Triple::ArchType::aarch64 ||
+   machine == llvm::Triple::ArchType::aarch64_be ||
+   machine == llvm::Triple::ArchType::aarch64_32))

If you passed in a `llvm::Triple`, you could replace this with 
`triple.isAArch64()`.

And one could imagine that e.g. the environment field of the triple could be 
important in determining the precise unwind plan.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:900
   // section, so prefer that if available. On other platforms we may need to
   // provide a platform-specific UnwindPlan which encodes the details of how to
   // unwind out of sigtramp.

labath wrote:
> this comment
I guess we should also update this comment now. Maybe:
```
On some platforms the unwind information for signal handlers is not present or 
correct. Give the platform plugins a chance to provide substitute plan. 
Otherwise, use eh_frame.
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:295
+  unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes);
+  unwind_plan_sp->SetRegisterKind(eRegisterKindDWARF);
+

Ah now I see that it worked because of this. Maybe you could try dropping this 
line and replace the dwarf numbers with generic regnums?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

[Lldb-commits] [lldb] f5158ca - Modernize Platform::GetOSKernelDescription

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-27T10:46:47+02:00
New Revision: f5158ca48c260dd29136ab19ba8573226f087fb3

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

LOG: Modernize Platform::GetOSKernelDescription

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/RemoteAwarePlatform.h
lldb/source/API/SBPlatform.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Target/Platform.cpp
lldb/source/Target/RemoteAwarePlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index adf6d865ab184..1106ce868761c 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -214,7 +214,7 @@ class Platform : public PluginInterface {
 
   llvm::Optional GetOSBuildString();
 
-  bool GetOSKernelDescription(std::string &s);
+  llvm::Optional GetOSKernelDescription();
 
   // Returns the name of the platform
   ConstString GetName();
@@ -244,9 +244,8 @@ class Platform : public PluginInterface {
 return llvm::None;
   }
 
-  virtual bool GetRemoteOSKernelDescription(std::string &s) {
-s.clear();
-return false;
+  virtual llvm::Optional GetRemoteOSKernelDescription() {
+return llvm::None;
   }
 
   // Remote Platform subclasses need to override this function

diff  --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index d8f7720d2fd96..f2a4ffae2aae3 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -65,7 +65,7 @@ class RemoteAwarePlatform : public Platform {
 
   bool GetRemoteOSVersion() override;
   llvm::Optional GetRemoteOSBuildString() override;
-  bool GetRemoteOSKernelDescription(std::string &s) override;
+  llvm::Optional GetRemoteOSKernelDescription() override;
   ArchSpec GetRemoteSystemArchitecture() override;
 
   Status RunShellCommand(llvm::StringRef command, const FileSpec &working_dir,

diff  --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 71d6b1c41e32f..d7a86f0ad1ddc 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -473,13 +473,11 @@ const char *SBPlatform::GetOSDescription() {
 
   PlatformSP platform_sp(GetSP());
   if (platform_sp) {
-std::string s;
-if (platform_sp->GetOSKernelDescription(s)) {
-  if (!s.empty()) {
-// Const-ify the string so we don't need to worry about the lifetime of
-// the string
-return ConstString(s.c_str()).GetCString();
-  }
+std::string s = platform_sp->GetOSKernelDescription().getValueOr("");
+if (!s.empty()) {
+  // Const-ify the string so we don't need to worry about the lifetime of
+  // the string
+  return ConstString(s.c_str()).GetCString();
 }
   }
   return nullptr;

diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 851d6291591a8..d0a8fc4ebf381 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -240,8 +240,9 @@ llvm::Optional 
PlatformRemoteGDBServer::GetRemoteOSBuildString() {
   return m_gdb_client.GetOSBuildString();
 }
 
-bool PlatformRemoteGDBServer::GetRemoteOSKernelDescription(std::string &s) {
-  return m_gdb_client.GetOSKernelDescription(s);
+llvm::Optional
+PlatformRemoteGDBServer::GetRemoteOSKernelDescription() {
+  return m_gdb_client.GetOSKernelDescription();
 }
 
 // Remote Platform subclasses need to override this function

diff  --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
index b5e52e0219828..f3d7ef17ccabb 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
@@ -79,7 +79,7 @@ class PlatformRemoteGDBServer : public Platform, private 
UserIDResolver {
 
   llvm::Optional GetRemoteOSBuildString() override;
 
-  bool GetRemoteOSKernelDescription(std::string &s) override;
+  llvm::Optional GetRemoteOSKernelDescription() override;
 
   // Remote Platform subclasses need to override this function
   ArchSpec GetRemoteSystemArchitecture() override;

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemote

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 382575.
DavidSpickett added a comment.

Turns out comparing thread.get_thread_frames() doesn't work because
you end up getting references to the same frames. Instead save the info
we want to compare.

One obvious reason this tactic should have failed is that the libc raise
frame will have a different ID between the two backtraces. This is why I've
ignored ID in the comparion.

Doing this showed me that sp and fp should be set, so I've got fp from
the sigcontext also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py

Index: lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
===
--- lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
+++ lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
@@ -7,7 +7,21 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from collections import namedtuple
+
+# Since frames internally point to ExecutionContextRef which itself points
+# to something else, we can't make a deep copy of them from Python.
+# Instead save what we care about for comparison purposes.
+# Ignoring the frame ID because what is frame 0 in the first catch will
+# be frame 2 in the handler backtrace.
+class FrameInfo(namedtuple('FrameInfo', ['sp', 'fp', 'function_name'])):
+# So assert failures are more readable
+def __repr__(self):
+return "SP: 0x{:x} FP: 0x{:x} Fn: {}".format(
+self.sp, self.fp, self.function_name)
+
+def make_frame_info(frame):
+return FrameInfo(frame.GetSP(), frame.GetFP(), frame.GetDisplayFunctionName())
 
 class HandleAbortTestCase(TestBase):
 
@@ -16,8 +30,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows  # signals do not exist on Windows
-# Fails on Ubuntu Focal
-@skipIf(archs=["aarch64"], oslist=["linux"])
 @expectedFailureNetBSD
 def test_inferior_handle_sigabrt(self):
 """Inferior calls abort() and handles the resultant SIGABRT.
@@ -47,6 +59,9 @@
 self.assertEqual(thread.GetStopReasonDataAtIndex(0),
  signo, "The stop signal should be SIGABRT")
 
+# Save the backtrace frames to compare to the handler backtrace later.
+signal_frames = [make_frame_info(f) for f in thread.get_thread_frames()]
+
 # Continue to breakpoint in abort handler
 bkpt = target.FindBreakpointByID(
 lldbutil.run_break_set_by_source_regexp(self, "Set a breakpoint here"))
@@ -59,12 +74,20 @@
 self.assertEqual(frame.GetDisplayFunctionName(), "handler", "Unexpected break?")
 
 # Expect that unwinding should find 'abort_caller'
-foundFoo = False
-for frame in thread:
+found_caller = False
+for frame in thread.get_thread_frames():
 if frame.GetDisplayFunctionName() == "abort_caller":
-foundFoo = True
+found_caller = True
+break
+
+self.assertTrue(found_caller, "Unwinding did not find func that called abort")
+
+# The signal handler backtrace has extra frames at the start, remove those
+handler_frames = thread.get_thread_frames()[-len(signal_frames):]
+handler_frames = [make_frame_info(f) for f in handler_frames]
 
-self.assertTrue(foundFoo, "Unwinding did not find func that called abort")
+# Check that frames present in both backtraces have the same addresses.
+self.assertEqual(signal_frames, handler_frames, "Common backtrace frames do not match")
 
 # Continue until we exit.
 process.Continue()
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -900,6 +900,17 @@
   // unwind out of sigtramp.
   if (m_frame_type == eTrapHandlerFrame && process) {
 m_fast_unwind_plan_sp.reset();
+
+if (m_sym_ctx_valid) {
+  lldb::PlatformSP platform = process->GetTarget().GetPlatform();
+  unwind_plan_sp = platform->GetTrapHandlerUnwindPlan(
+  process->GetTarget().GetArchitecture().GetMachine(),
+  GetSymbolOrFunctionName(m_sym_ctx));
+
+  if (unwind_plan_sp)
+return unwind_plan_sp;
+}
+
 unwind_plan_sp =
 func_unwinders_sp->GetEHFrameUnwindPlan(process->GetTarget());
 if (!unwind_plan_sp)
Index: lldb/source/Plugins/Platform/Linux/PlatformLinux.h
=

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Thanks for doing this. Just a couple of comments inline.

Will respond to these shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Since the signal context contains a copy of all registers, maybe you should be 
setting all of them. The other registers will not be used for unwinding, but 
they will enable to user to inspect the registers in the stack frames above the 
signal handler.
One way to demonstrate (and test) this would be to have a bit of inline asm, 
which sets all registers to known values, and then raises a signal (SIGILL is 
probably the easiest to raise from asm). Then lldb breaks on the signal 
handler, goes up the stack, and observes the registers.




Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:295
+  unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes);
+  unwind_plan_sp->SetRegisterKind(eRegisterKindDWARF);
+

labath wrote:
> Ah now I see that it worked because of this. Maybe you could try dropping 
> this line and replace the dwarf numbers with generic regnums?
Except of course, if you end up setting all of the registers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 382590.
DavidSpickett added a comment.

Use generic register names and don't set type to DWARF.

Pass triple instead of machine and use isAArch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py

Index: lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
===
--- lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
+++ lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
@@ -7,7 +7,21 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from collections import namedtuple
+
+# Since frames internally point to ExecutionContextRef which itself points
+# to something else, we can't make a deep copy of them from Python.
+# Instead save what we care about for comparison purposes.
+# Ignoring the frame ID because what is frame 0 in the first catch will
+# be frame 2 in the handler backtrace.
+class FrameInfo(namedtuple('FrameInfo', ['sp', 'fp', 'function_name'])):
+# So assert failures are more readable
+def __repr__(self):
+return "SP: 0x{:x} FP: 0x{:x} Fn: {}".format(
+self.sp, self.fp, self.function_name)
+
+def make_frame_info(frame):
+return FrameInfo(frame.GetSP(), frame.GetFP(), frame.GetDisplayFunctionName())
 
 class HandleAbortTestCase(TestBase):
 
@@ -16,8 +30,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows  # signals do not exist on Windows
-# Fails on Ubuntu Focal
-@skipIf(archs=["aarch64"], oslist=["linux"])
 @expectedFailureNetBSD
 def test_inferior_handle_sigabrt(self):
 """Inferior calls abort() and handles the resultant SIGABRT.
@@ -47,6 +59,9 @@
 self.assertEqual(thread.GetStopReasonDataAtIndex(0),
  signo, "The stop signal should be SIGABRT")
 
+# Save the backtrace frames to compare to the handler backtrace later.
+signal_frames = [make_frame_info(f) for f in thread.get_thread_frames()]
+
 # Continue to breakpoint in abort handler
 bkpt = target.FindBreakpointByID(
 lldbutil.run_break_set_by_source_regexp(self, "Set a breakpoint here"))
@@ -59,12 +74,20 @@
 self.assertEqual(frame.GetDisplayFunctionName(), "handler", "Unexpected break?")
 
 # Expect that unwinding should find 'abort_caller'
-foundFoo = False
-for frame in thread:
+found_caller = False
+for frame in thread.get_thread_frames():
 if frame.GetDisplayFunctionName() == "abort_caller":
-foundFoo = True
+found_caller = True
+break
+
+self.assertTrue(found_caller, "Unwinding did not find func that called abort")
+
+# The signal handler backtrace has extra frames at the start, remove those
+handler_frames = thread.get_thread_frames()[-len(signal_frames):]
+handler_frames = [make_frame_info(f) for f in handler_frames]
 
-self.assertTrue(foundFoo, "Unwinding did not find func that called abort")
+# Check that frames present in both backtraces have the same addresses.
+self.assertEqual(signal_frames, handler_frames, "Common backtrace frames do not match")
 
 # Continue until we exit.
 process.Continue()
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -900,6 +900,17 @@
   // unwind out of sigtramp.
   if (m_frame_type == eTrapHandlerFrame && process) {
 m_fast_unwind_plan_sp.reset();
+
+if (m_sym_ctx_valid) {
+  lldb::PlatformSP platform = process->GetTarget().GetPlatform();
+  unwind_plan_sp = platform->GetTrapHandlerUnwindPlan(
+  process->GetTarget().GetArchitecture().GetTriple(),
+  GetSymbolOrFunctionName(m_sym_ctx));
+
+  if (unwind_plan_sp)
+return unwind_plan_sp;
+}
+
 unwind_plan_sp =
 func_unwinders_sp->GetEHFrameUnwindPlan(process->GetTarget());
 if (!unwind_plan_sp)
Index: lldb/source/Plugins/Platform/Linux/PlatformLinux.h
===
--- lldb/source/Plugins/Platform/Linux/PlatformLinux.h
+++ lldb/source/Plugins/Platform/Linux/PlatformLinux.h
@@ -50,6 +50,9 @@
 
   void CalculateTrapHandlerSymbolNames() override;
 
+  lldb::UnwindPlanSP GetTrapHandlerUnwindPlan(const llvm::Triple &triple,
+   

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 382591.
DavidSpickett added a comment.

Update comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py

Index: lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
===
--- lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
+++ lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
@@ -7,7 +7,21 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from collections import namedtuple
+
+# Since frames internally point to ExecutionContextRef which itself points
+# to something else, we can't make a deep copy of them from Python.
+# Instead save what we care about for comparison purposes.
+# Ignoring the frame ID because what is frame 0 in the first catch will
+# be frame 2 in the handler backtrace.
+class FrameInfo(namedtuple('FrameInfo', ['sp', 'fp', 'function_name'])):
+# So assert failures are more readable
+def __repr__(self):
+return "SP: 0x{:x} FP: 0x{:x} Fn: {}".format(
+self.sp, self.fp, self.function_name)
+
+def make_frame_info(frame):
+return FrameInfo(frame.GetSP(), frame.GetFP(), frame.GetDisplayFunctionName())
 
 class HandleAbortTestCase(TestBase):
 
@@ -16,8 +30,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows  # signals do not exist on Windows
-# Fails on Ubuntu Focal
-@skipIf(archs=["aarch64"], oslist=["linux"])
 @expectedFailureNetBSD
 def test_inferior_handle_sigabrt(self):
 """Inferior calls abort() and handles the resultant SIGABRT.
@@ -47,6 +59,9 @@
 self.assertEqual(thread.GetStopReasonDataAtIndex(0),
  signo, "The stop signal should be SIGABRT")
 
+# Save the backtrace frames to compare to the handler backtrace later.
+signal_frames = [make_frame_info(f) for f in thread.get_thread_frames()]
+
 # Continue to breakpoint in abort handler
 bkpt = target.FindBreakpointByID(
 lldbutil.run_break_set_by_source_regexp(self, "Set a breakpoint here"))
@@ -59,12 +74,20 @@
 self.assertEqual(frame.GetDisplayFunctionName(), "handler", "Unexpected break?")
 
 # Expect that unwinding should find 'abort_caller'
-foundFoo = False
-for frame in thread:
+found_caller = False
+for frame in thread.get_thread_frames():
 if frame.GetDisplayFunctionName() == "abort_caller":
-foundFoo = True
+found_caller = True
+break
+
+self.assertTrue(found_caller, "Unwinding did not find func that called abort")
+
+# The signal handler backtrace has extra frames at the start, remove those
+handler_frames = thread.get_thread_frames()[-len(signal_frames):]
+handler_frames = [make_frame_info(f) for f in handler_frames]
 
-self.assertTrue(foundFoo, "Unwinding did not find func that called abort")
+# Check that frames present in both backtraces have the same addresses.
+self.assertEqual(signal_frames, handler_frames, "Common backtrace frames do not match")
 
 # Continue until we exit.
 process.Continue()
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -893,13 +893,22 @@
 return arch_default_unwind_plan_sp;
   }
 
-  // If we're in _sigtramp(), unwinding past this frame requires special
-  // knowledge.  On Mac OS X this knowledge is properly encoded in the eh_frame
-  // section, so prefer that if available. On other platforms we may need to
-  // provide a platform-specific UnwindPlan which encodes the details of how to
-  // unwind out of sigtramp.
   if (m_frame_type == eTrapHandlerFrame && process) {
 m_fast_unwind_plan_sp.reset();
+
+// On some platforms the unwind information for signal handlers is not
+// present or correct. Give the platform plugins a chance to provide
+// substitute plan. Otherwise, use eh_frame.
+if (m_sym_ctx_valid) {
+  lldb::PlatformSP platform = process->GetTarget().GetPlatform();
+  unwind_plan_sp = platform->GetTrapHandlerUnwindPlan(
+  process->GetTarget().GetArchitecture().GetTriple(),
+  GetSymbolOrFunctionName(m_sym_ctx));
+
+  if (unwind_plan_sp)
+return unwind_plan_sp;
+}
+
 unwind_plan_sp =
 func_unwinders_sp->GetEHFrameU

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> One way to demonstrate (and test) this would be to have a bit of inline asm, 
> which sets all registers to known values, and then raises a signal (SIGILL is 
> probably the easiest to raise from asm).

I will give that a go, probably as a separate test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

[Lldb-commits] [lldb] e1acadb - [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-27T12:45:52+02:00
New Revision: e1acadb61dfc0810656219c6314019d5132f2c61

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

LOG: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

Refactor ConnectionFileDescriptor to improve code reuse for different
types of sockets.  Unify method naming.

While at it, remove some (now-)dead code from Socket.

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

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 780c6e6a1..306763025d4bc 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -75,18 +75,6 @@ class Socket : public IOObject {
   static llvm::Expected>
   UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
 
-  static Status UnixDomainConnect(llvm::StringRef host_and_port,
-  bool child_processes_inherit,
-  Socket *&socket);
-  static Status UnixDomainAccept(llvm::StringRef host_and_port,
- bool child_processes_inherit, Socket 
*&socket);
-  static Status UnixAbstractConnect(llvm::StringRef host_and_port,
-bool child_processes_inherit,
-Socket *&socket);
-  static Status UnixAbstractAccept(llvm::StringRef host_and_port,
-   bool child_processes_inherit,
-   Socket *&socket);
-
   int GetOption(int level, int option_name, int &option_value);
   int SetOption(int level, int option_name, int option_value);
 

diff  --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 12eab5fc8ae83..35773d5907e91 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -16,6 +16,7 @@
 #include "lldb/lldb-forward.h"
 
 #include "lldb/Host/Pipe.h"
+#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Connection.h"
 #include "lldb/Utility/IOObject.h"
 
@@ -73,9 +74,18 @@ class ConnectionFileDescriptor : public Connection {
   void CloseCommandPipe();
 
   lldb::ConnectionStatus
-  SocketListenAndAccept(llvm::StringRef host_and_port,
-socket_id_callback_type socket_id_callback,
-Status *error_ptr);
+  AcceptSocket(Socket::SocketProtocol socket_protocol,
+   llvm::StringRef socket_name,
+   llvm::function_ref post_listen_callback,
+   Status *error_ptr);
+
+  lldb::ConnectionStatus ConnectSocket(Socket::SocketProtocol socket_protocol,
+   llvm::StringRef socket_name,
+   Status *error_ptr);
+
+  lldb::ConnectionStatus AcceptTCP(llvm::StringRef host_and_port,
+   socket_id_callback_type socket_id_callback,
+   Status *error_ptr);
 
   lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
 socket_id_callback_type socket_id_callback,
@@ -86,24 +96,24 @@ class ConnectionFileDescriptor : public Connection {
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  NamedSocketConnect(llvm::StringRef socket_name,
+  ConnectNamedSocket(llvm::StringRef socket_name,
  socket_id_callback_type socket_id_callback,
  Status *error_ptr);
 
   lldb::ConnectionStatus
-  NamedSocketAccept(llvm::StringRef socket_name,
+  AcceptNamedSocket(llvm::StringRef socket_name,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  UnixAbstractSocketAccept(llvm::StringRef socket_name,
-   socket_id_callback_type socket_id_callback,
-   Status *error_ptr);
+  AcceptAbstractSocket(llvm::StringRef socket_name,
+   socket_id_callback_type socket_id_callback,
+   Status *error_ptr);
 
   lldb::ConnectionStatus
-  UnixAbstractSocketConnect(llvm::StringRef socket_name,
-socket_id_callback_type socket_id_callback,
-Status *error_ptr);
+  ConnectAbstractSocket(llvm::StringRef socket_name,
+socket_id_callback_type socket_id_callback,
+ 

[Lldb-commits] [PATCH] D112495: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Michał Górny 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 rGe1acadb61dfc: [lldb] [Host/ConnectionFileDescriptor] 
Refactor to improve code reuse (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112495

Files:
  lldb/include/lldb/Host/Socket.h
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -151,17 +151,17 @@
 auto method =
 llvm::StringSwitch(scheme)
-.Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
+.Case("listen", &ConnectionFileDescriptor::AcceptTCP)
 .Cases("accept", "unix-accept",
-   &ConnectionFileDescriptor::NamedSocketAccept)
+   &ConnectionFileDescriptor::AcceptNamedSocket)
 .Case("unix-abstract-accept",
-  &ConnectionFileDescriptor::UnixAbstractSocketAccept)
+  &ConnectionFileDescriptor::AcceptAbstractSocket)
 .Cases("connect", "tcp-connect",
&ConnectionFileDescriptor::ConnectTCP)
 .Case("udp", &ConnectionFileDescriptor::ConnectUDP)
-.Case("unix-connect", &ConnectionFileDescriptor::NamedSocketConnect)
+.Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket)
 .Case("unix-abstract-connect",
-  &ConnectionFileDescriptor::UnixAbstractSocketConnect)
+  &ConnectionFileDescriptor::ConnectAbstractSocket)
 #if LLDB_ENABLE_POSIX
 .Case("fd", &ConnectionFileDescriptor::ConnectFD)
 .Case("file", &ConnectionFileDescriptor::ConnectFile)
@@ -532,24 +532,25 @@
   return eConnectionStatusLostConnection;
 }
 
-ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
-llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+lldb::ConnectionStatus ConnectionFileDescriptor::AcceptSocket(
+Socket::SocketProtocol socket_protocol, llvm::StringRef socket_name,
+llvm::function_ref post_listen_callback,
 Status *error_ptr) {
   Status error;
-  std::unique_ptr listen_socket = Socket::Create(
-  Socket::ProtocolUnixDomain, m_child_processes_inherit, error);
-  Socket *socket = nullptr;
+  std::unique_ptr listening_socket =
+  Socket::Create(socket_protocol, m_child_processes_inherit, error);
+  Socket *accepted_socket;
 
   if (!error.Fail())
-error = listen_socket->Listen(socket_name, 5);
+error = listening_socket->Listen(socket_name, 5);
 
   if (!error.Fail()) {
-socket_id_callback(socket_name);
-error = listen_socket->Accept(socket);
+post_listen_callback(*listening_socket);
+error = listening_socket->Accept(accepted_socket);
   }
 
   if (!error.Fail()) {
-m_io_sp.reset(socket);
+m_io_sp.reset(accepted_socket);
 m_uri.assign(socket_name.str());
 return eConnectionStatusSuccess;
   }
@@ -559,40 +560,19 @@
   return eConnectionStatusError;
 }
 
-ConnectionStatus ConnectionFileDescriptor::NamedSocketConnect(
-llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
-Status *error_ptr) {
-  Socket *socket = nullptr;
-  Status error =
-  Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket);
-  if (error_ptr)
-*error_ptr = error;
-  m_io_sp.reset(socket);
-  if (error.Fail())
-return eConnectionStatusError;
-  m_uri.assign(std::string(socket_name));
-  return eConnectionStatusSuccess;
-}
-
-ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
-llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
-Status *error_ptr) {
+lldb::ConnectionStatus
+ConnectionFileDescriptor::ConnectSocket(Socket::SocketProtocol socket_protocol,
+llvm::StringRef socket_name,
+Status *error_ptr) {
   Status error;
-  std::unique_ptr listen_socket = Socket::Create(
-  Socket::ProtocolUnixAbstract, m_child_processes_inherit, error);
-  Socket *socket = nullptr;
-
-  if (!error.Fail())
-error = listen_socket->Listen(socket_name, 5);
+  std::unique_ptr socket =
+  Socket::Create(socket_protocol, m_child_processes_inherit, error);
 
   if (!error.Fail())
-socket_id_callback(socket_name);
-
-  if (!error.Fail())
-error = listen_socket->Accept(socket);
+error = socket->Connect(socket_name);
 
   if (!error.Fail()) {
-m_io_sp.reset(socket);
+m_io_sp = std::move(socket);
 m_uri.assign(socket_nam

[Lldb-commits] [lldb] 560221a - [lldb] Modernize TestVLA.py

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-27T12:47:56+02:00
New Revision: 560221ac7f5ca3d5dcae405587db066f4c4c8a7c

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

LOG: [lldb] Modernize TestVLA.py

Use expect_expr/var_path instead of regular expect and substring checks

Added: 


Modified: 
lldb/test/API/lang/c/vla/TestVLA.py

Removed: 




diff  --git a/lldb/test/API/lang/c/vla/TestVLA.py 
b/lldb/test/API/lang/c/vla/TestVLA.py
index 215eadc41a739..758406d72e635 100644
--- a/lldb/test/API/lang/c/vla/TestVLA.py
+++ b/lldb/test/API/lang/c/vla/TestVLA.py
@@ -32,14 +32,19 @@ def test_vla(self):
 _, process, _, _ = lldbutil.run_to_source_breakpoint(
 self, "break here", lldb.SBFileSpec('main.c'))
 
-def test(a, array):
+def test(a):
+children = []
 for i in range(a):
-self.expect("fr v vla[%d]"%i, substrs=["int", "%d"%(a-i)])
-self.expect("expr vla[%d]"%i, substrs=["int", "%d"%(a-i)])
-self.expect("fr v vla", substrs=array)
+name = "[%d]"%i
+value = str(a-i)
+self.expect_var_path("vla"+name, type="int", value=value)
+self.expect_expr("vla"+name, result_type="int",
+result_value=value)
+children.append(ValueCheck(name=name, value=value))
+self.expect_var_path("vla", type="int[]", children=children)
 self.expect("expr vla", error=True, substrs=["incomplete"])
 
-test(2, ["int[]", "[0] = 2, [1] = 1"])
+test(2)
 process.Continue()
-test(4, ["int[]", "[0] = 4, [1] = 3, [2] = 2, [3] = 1"])
+test(4)
 



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

[Lldb-commits] [PATCH] D112612: [lldb] [docs] Remove obsolete recommonmark use

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor.
mgorny requested review of this revision.

The recommonmark package is no longer required since all the documents
have been converted to .rst.  Remove the related support code from
docs/conf.py.


https://reviews.llvm.org/D112612

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -61,21 +61,6 @@
 '.rst': 'restructuredtext',
 }
 
-try:
-  import recommonmark
-except ImportError:
-  # manpages do not use any .md sources
-  if not building_man_page:
-raise
-else:
-  import sphinx
-  if sphinx.version_info >= (3, 0):
-# This requires 0.5 or later.
-extensions.append('recommonmark')
-  else:
-source_parsers = {'.md': 'recommonmark.parser.CommonMarkParser'}
-  source_suffix['.md'] = 'markdown'
-
 # The encoding of source files.
 #source_encoding = 'utf-8-sig'
 
@@ -285,20 +270,6 @@
 # If true, show URL addresses after external links.
 #man_show_urls = False
 
-def process_md(name):
-file_subpath = os.path.join(command_guide_subpath, name)
-with open(os.path.join(command_guide_path, name)) as f:
-title = f.readline().rstrip('\n')
-
-m = re.match(r'^# (\S+) - (.+)$', title)
-if m is None:
-print("error: invalid title in %r "
-  "(expected '#  - ')" % file_subpath,
-  file=sys.stderr)
-else:
-man_pages.append((file_subpath.replace('.md',''), m.group(1),
-  m.group(2), man_page_authors, 1))
-
 # -- Options for Texinfo output 

 
 # Grouping the document tree into Texinfo files. List of tuples


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -61,21 +61,6 @@
 '.rst': 'restructuredtext',
 }
 
-try:
-  import recommonmark
-except ImportError:
-  # manpages do not use any .md sources
-  if not building_man_page:
-raise
-else:
-  import sphinx
-  if sphinx.version_info >= (3, 0):
-# This requires 0.5 or later.
-extensions.append('recommonmark')
-  else:
-source_parsers = {'.md': 'recommonmark.parser.CommonMarkParser'}
-  source_suffix['.md'] = 'markdown'
-
 # The encoding of source files.
 #source_encoding = 'utf-8-sig'
 
@@ -285,20 +270,6 @@
 # If true, show URL addresses after external links.
 #man_show_urls = False
 
-def process_md(name):
-file_subpath = os.path.join(command_guide_subpath, name)
-with open(os.path.join(command_guide_path, name)) as f:
-title = f.readline().rstrip('\n')
-
-m = re.match(r'^# (\S+) - (.+)$', title)
-if m is None:
-print("error: invalid title in %r "
-  "(expected '#  - ')" % file_subpath,
-  file=sys.stderr)
-else:
-man_pages.append((file_subpath.replace('.md',''), m.group(1),
-  m.group(2), man_page_authors, 1))
-
 # -- Options for Texinfo output 
 
 # Grouping the document tree into Texinfo files. List of tuples
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d03b04f - [lldb] [docs] Remove obsolete recommonmark use

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-27T13:45:05+02:00
New Revision: d03b04f211e73c2f59ba5dc6a6a8c777de001ad6

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

LOG: [lldb] [docs] Remove obsolete recommonmark use

The recommonmark package is no longer required since all the documents
have been converted to .rst.  Remove the related support code from
docs/conf.py.

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

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 0c3cd59fc11ef..cf3af568572b6 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -61,21 +61,6 @@
 '.rst': 'restructuredtext',
 }
 
-try:
-  import recommonmark
-except ImportError:
-  # manpages do not use any .md sources
-  if not building_man_page:
-raise
-else:
-  import sphinx
-  if sphinx.version_info >= (3, 0):
-# This requires 0.5 or later.
-extensions.append('recommonmark')
-  else:
-source_parsers = {'.md': 'recommonmark.parser.CommonMarkParser'}
-  source_suffix['.md'] = 'markdown'
-
 # The encoding of source files.
 #source_encoding = 'utf-8-sig'
 
@@ -285,20 +270,6 @@
 # If true, show URL addresses after external links.
 #man_show_urls = False
 
-def process_md(name):
-file_subpath = os.path.join(command_guide_subpath, name)
-with open(os.path.join(command_guide_path, name)) as f:
-title = f.readline().rstrip('\n')
-
-m = re.match(r'^# (\S+) - (.+)$', title)
-if m is None:
-print("error: invalid title in %r "
-  "(expected '#  - ')" % file_subpath,
-  file=sys.stderr)
-else:
-man_pages.append((file_subpath.replace('.md',''), m.group(1),
-  m.group(2), man_page_authors, 1))
-
 # -- Options for Texinfo output 

 
 # Grouping the document tree into Texinfo files. List of tuples



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

[Lldb-commits] [PATCH] D112612: [lldb] [docs] Remove obsolete recommonmark use

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd03b04f211e7: [lldb] [docs] Remove obsolete recommonmark use 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112612

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -61,21 +61,6 @@
 '.rst': 'restructuredtext',
 }
 
-try:
-  import recommonmark
-except ImportError:
-  # manpages do not use any .md sources
-  if not building_man_page:
-raise
-else:
-  import sphinx
-  if sphinx.version_info >= (3, 0):
-# This requires 0.5 or later.
-extensions.append('recommonmark')
-  else:
-source_parsers = {'.md': 'recommonmark.parser.CommonMarkParser'}
-  source_suffix['.md'] = 'markdown'
-
 # The encoding of source files.
 #source_encoding = 'utf-8-sig'
 
@@ -285,20 +270,6 @@
 # If true, show URL addresses after external links.
 #man_show_urls = False
 
-def process_md(name):
-file_subpath = os.path.join(command_guide_subpath, name)
-with open(os.path.join(command_guide_path, name)) as f:
-title = f.readline().rstrip('\n')
-
-m = re.match(r'^# (\S+) - (.+)$', title)
-if m is None:
-print("error: invalid title in %r "
-  "(expected '#  - ')" % file_subpath,
-  file=sys.stderr)
-else:
-man_pages.append((file_subpath.replace('.md',''), m.group(1),
-  m.group(2), man_page_authors, 1))
-
 # -- Options for Texinfo output 

 
 # Grouping the document tree into Texinfo files. List of tuples


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -61,21 +61,6 @@
 '.rst': 'restructuredtext',
 }
 
-try:
-  import recommonmark
-except ImportError:
-  # manpages do not use any .md sources
-  if not building_man_page:
-raise
-else:
-  import sphinx
-  if sphinx.version_info >= (3, 0):
-# This requires 0.5 or later.
-extensions.append('recommonmark')
-  else:
-source_parsers = {'.md': 'recommonmark.parser.CommonMarkParser'}
-  source_suffix['.md'] = 'markdown'
-
 # The encoding of source files.
 #source_encoding = 'utf-8-sig'
 
@@ -285,20 +270,6 @@
 # If true, show URL addresses after external links.
 #man_show_urls = False
 
-def process_md(name):
-file_subpath = os.path.join(command_guide_subpath, name)
-with open(os.path.join(command_guide_path, name)) as f:
-title = f.readline().rstrip('\n')
-
-m = re.match(r'^# (\S+) - (.+)$', title)
-if m is None:
-print("error: invalid title in %r "
-  "(expected '#  - ')" % file_subpath,
-  file=sys.stderr)
-else:
-man_pages.append((file_subpath.replace('.md',''), m.group(1),
-  m.group(2), man_page_authors, 1))
-
 # -- Options for Texinfo output 
 
 # Grouping the document tree into Texinfo files. List of tuples
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [lldb] 99f5f0a - fix comment typos to cycle bots

[Nico Weber via lldb-commits]

Author: Nico Weber
Date: 2021-10-27T09:43:42-04:00
New Revision: 99f5f0a2b72bfaf0bfb527466fdd50748583559f

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

LOG: fix comment typos to cycle bots

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 205b03464534..8d9b9bf86e31 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1724,7 +1724,7 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
 DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
   // If this is a Darwin-style debug map (non-.dSYM) symbol file,
   // never attempt to load ELF-style DWO files since the -gmodules
-  // support uses the same DWO machanism to specify full debug info
+  // support uses the same DWO mechanism to specify full debug info
   // files for modules. This is handled in
   // UpdateExternalModuleListIfNeeded().
   if (GetDebugMapSymfile())
@@ -2889,7 +2889,7 @@ TypeSP 
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(
   }
 
   m_index->GetTypes(dwarf_decl_ctx, [&](DWARFDIE type_die) {
-// Make sure type_die's langauge matches the type system we are
+// Make sure type_die's language matches the type system we are
 // looking for. We don't want to find a "Foo" type from Java if we
 // are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
 if (type_system &&
@@ -3351,7 +3351,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext &sc,
 SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
 if (debug_map_symfile)
   // Set the module of the expression to the linked module
-  // instead of the oject file so the relocated address can be
+  // instead of the object file so the relocated address can be
   // found there.
   location.SetModule(debug_map_symfile->GetObjectFile()->GetModule());
 
@@ -3626,7 +3626,7 @@ SymbolFileDWARF::MergeBlockAbstractParameters(const 
DWARFDIE &block_die,
   DWARFDIE origin_of_concrete =
   GetDIE(*concrete_it).GetReferencedDIE(DW_AT_abstract_origin);
   if (origin_of_concrete == abstract_child) {
-// The current abstract paramater is the origin of the current
+// The current abstract parameter is the origin of the current
 // concrete parameter, just push the concrete parameter.
 merged.push_back(*concrete_it);
 ++concrete_it;



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

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan updated this revision to Diff 382665.
danilashtefan edited the summary of this revision.
danilashtefan added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-#include 
-#include 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set &ref, std::set *ptr)
-{
-// Stop here to check by ref and ptr
-return;
-} 
-
-int main()
-{
-std::set ii;
-thefoo_rw(1);  // Set break point at this line.
-	
-	ii.insert(0);
-	ii.insert(1);
-	ii.insert(2);
-	ii.insert(3);
-	ii.insert(4);
-	ii.insert(5);
-thefoo_rw(1);  // Set break point at this line.
-
-	ii.insert(6);
-	thefoo_rw(1);  // Set break point at this line.
-
-by_ref_and_ptr(ii, &ii);
-
-	ii.clear();
-	thefoo_rw(1);  // Set break point at this line.
-
-	std::set ss;
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("a");
-	ss.insert("a very long string is right here");
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("b");
-	ss.insert("c");
-	thefoo_rw(1);  // Set break point at this line.
-	
-	ss.erase("b");
-	thefoo_rw(1);  // Set break point at this line.
-
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
@@ -0,0 +1,55 @@
+#include 
+#include 
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+arg = 0;
+  if (!arg)
+arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set &ref, std::set *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
@@ -9,6 +9,9 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
+
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
@@ -18,34 +21,29 @@
 TestBase.setUp(self)
 self.namespace = 'std'
 
-def getVariableType(self, name):
+def findVariable(self, name):
 var = self.frame().FindVariable(name)
 self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
 return var.GetType().GetDisplayTypeName()
 
-def check_ii(self, var_name):
-""" This checks the value of the bitset stored in ii at the call to by_ref_and_ptr.
-We use this to make sure we get the same values for ii when we look at the object
-directly, and when we look at a reference to the object. """
-self.expect(
-"frame variable " 

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py:45
 "[3] = 3",
 "[6] = 6"])
 

wallace wrote:
> can you add additional assertions like the one you did in 
> https://reviews.llvm.org/D112180 that uses ValueCheck?
I have made check method more generic and less verbose. Before it was 
specifically checking the "ii" variable, that was a set of size 7. With 
value check approach we can now test set and others. 

I guess, that I can delete all of the
```
self.expect() 
```
and simply call check method with the expected size. Please, let me know if I 
should do it or leave it as it is. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

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

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan updated this revision to Diff 382668.
danilashtefan added a comment.

Small formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-#include 
-#include 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set &ref, std::set *ptr)
-{
-// Stop here to check by ref and ptr
-return;
-} 
-
-int main()
-{
-std::set ii;
-thefoo_rw(1);  // Set break point at this line.
-	
-	ii.insert(0);
-	ii.insert(1);
-	ii.insert(2);
-	ii.insert(3);
-	ii.insert(4);
-	ii.insert(5);
-thefoo_rw(1);  // Set break point at this line.
-
-	ii.insert(6);
-	thefoo_rw(1);  // Set break point at this line.
-
-by_ref_and_ptr(ii, &ii);
-
-	ii.clear();
-	thefoo_rw(1);  // Set break point at this line.
-
-	std::set ss;
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("a");
-	ss.insert("a very long string is right here");
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("b");
-	ss.insert("c");
-	thefoo_rw(1);  // Set break point at this line.
-	
-	ss.erase("b");
-	thefoo_rw(1);  // Set break point at this line.
-
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
@@ -0,0 +1,55 @@
+#include 
+#include 
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+arg = 0;
+  if (!arg)
+arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set &ref, std::set *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
@@ -9,6 +9,8 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
@@ -18,34 +20,29 @@
 TestBase.setUp(self)
 self.namespace = 'std'
 
-def getVariableType(self, name):
+def findVariable(self, name):
 var = self.frame().FindVariable(name)
 self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
 return var.GetType().GetDisplayTypeName()
 
-def check_ii(self, var_name):
-""" This checks the value of the bitset stored in ii at the call to by_ref_and_ptr.
-We use this to make sure we get the same values for ii when we look at the object
-directly, and when we look at a reference to the object. """
-self.expect(
-"frame variable " + var_name,
-substrs=["size=7",

[Lldb-commits] [lldb] 9d7006c - [lldb][NFC] Move a declaration in DWARFASTParserClang to its first use.

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-10-27T17:46:50+02:00
New Revision: 9d7006c4aee693a4c09f18cecd244da919ebe13b

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

LOG: [lldb][NFC] Move a declaration in DWARFASTParserClang to its first use.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 0b3f83e70962..8affb05d56a2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2717,7 +2717,6 @@ void DWARFASTParserClang::ParseSingleMember(
 return;
   }
 
-  clang::FieldDecl *field_decl = nullptr;
   const uint64_t character_width = 8;
   const uint64_t word_width = 32;
   CompilerType member_clang_type = member_type->GetLayoutCompilerType();
@@ -2887,7 +2886,7 @@ void DWARFASTParserClang::ParseSingleMember(
 
   RequireCompleteType(member_clang_type);
 
-  field_decl = TypeSystemClang::AddFieldToRecordType(
+  clang::FieldDecl *field_decl = TypeSystemClang::AddFieldToRecordType(
   class_clang_type, attrs.name, member_clang_type, attrs.accessibility,
   attrs.bit_size);
 



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

[Lldb-commits] [PATCH] D112629: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste.
mgorny requested review of this revision.

https://reviews.llvm.org/D112629

Files:
  lldb/include/lldb/Host/Socket.h
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/tools/lldb-server/Acceptor.cpp
  lldb/unittests/Host/SocketTest.cpp

Index: lldb/unittests/Host/SocketTest.cpp
===
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -33,65 +33,40 @@
 };
 
 TEST_P(SocketTest, DecodeHostAndPort) {
-  std::string host_str;
-  std::string port_str;
-  uint16_t port;
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("localhost", host_str.c_str());
-  EXPECT_STREQ("1138", port_str.c_str());
-  EXPECT_EQ(1138, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("localhost:1138"),
+   llvm::HasValue(Socket::HostAndPort{"localhost", 1138}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:-1138"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:-1138'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("0", port_str.c_str());
-  EXPECT_EQ(0, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("*:65535", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("65535", port_str.c_str());
-  EXPECT_EQ(65535, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345",
-  host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("12345"),
+   llvm::HasValue(Socket::HostAndPort{"", 12345}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:0"),
+   llvm::HasValue(Socket::HostAndPort{"*", 0}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:65535"),
+   llvm::HasValue(Socket::HostAndPort{"*", 65535}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
 }
 
 #if LLDB_ENABLE_POSIX
Index: lldb/tools/lldb-server/Acceptor.cpp
===
--- lldb/tools/lldb-server/Acceptor.cpp
+++ lldb/tools/lldb-server/Acceptor.cpp
@@ -96,8 +96,7 @@
 std::string port_str;
 uint16_t port;
 // Try to match socket name as $host:port - e.g., localhost:
-if (!llvm::errorToBool(
-Socket::DecodeHostAndPort(name, host_str, port_str, port)))
+if (!llvm::errorToBool(Socket::DecodeHostAndPort(name).takeError()))
   socket_protocol = Socket::ProtocolTcp;
   }
 
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
@@ -3917,12 +3917,8 @@
   if (url_arg.startswith(":"))
 host_port.insert(0, "localhost");
 
-  std::

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[walter erquinigo via Phabricator via lldb-commits]

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

pretty good! Just some cosmetic changes needed and that's it




Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:318
 
-class StdMapSynthProvider:
+class StdSetOrMapSynthProvider:
 

Make a comment above like

  '''
  Set and Map have the same underlying data structure, therefore we can use 
exactly the same implementation for the formatter.
  '''



Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:324-325
 self.count = None
 logger >> "Providing synthetic children for a map named " + \
 str(valobj.GetName())
 

As this is not just for maps, you can do something like:

  self.kind = "set" if "set" in valobj.GetTypeName() else "map"
  logger >> "Providing synthetic children for a " self.kind + " named " + \
str(valobj.GetName())



Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:352
 try:
 # we will set this to True if we find out that discovering a node 
in the map takes more steps than the overall size of the RB tree
 # if this gets set to True, then we will merrily return None for





Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py:153-154
+self.do_test_ref_and_ptr(USE_LIBCPP)
+
+

remove these lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

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

[Lldb-commits] [PATCH] D111890: [lldb] [Host] Make Terminal methods return llvm::Error

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.
Herald added a subscriber: JDevlieghere.

This causes lots of warnings on Windows, where TERMIOS is false: 
http://45.33.8.238/win/47744/step_4.txt (look for "Terminal.cpp"). Could you 
take a look? It looks like return statements are genuinely missing there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111890

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

[Lldb-commits] [PATCH] D111890: [lldb] [Host] Make Terminal methods return llvm::Error

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D111890#3090632 , @thakis wrote:

> This causes lots of warnings on Windows, where TERMIOS is false: 
> http://45.33.8.238/win/47744/step_4.txt (look for "Terminal.cpp"). Could you 
> take a look? It looks like return statements are genuinely missing there.

When `LLDB_ENABLE_TERMIOS` is false, `GetData()` always returns an error, so 
the code below should not be reachable. I suppose I could add 
`llvm_unreachable()` for that. I'll try that and make a diff if it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111890

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

[Lldb-commits] [PATCH] D112629: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 382698.
mgorny added a comment.

Fix missing return in `operator<<`.


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

https://reviews.llvm.org/D112629

Files:
  lldb/include/lldb/Host/Socket.h
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/tools/lldb-server/Acceptor.cpp
  lldb/unittests/Host/SocketTest.cpp

Index: lldb/unittests/Host/SocketTest.cpp
===
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -33,65 +33,40 @@
 };
 
 TEST_P(SocketTest, DecodeHostAndPort) {
-  std::string host_str;
-  std::string port_str;
-  uint16_t port;
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("localhost", host_str.c_str());
-  EXPECT_STREQ("1138", port_str.c_str());
-  EXPECT_EQ(1138, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("localhost:1138"),
+   llvm::HasValue(Socket::HostAndPort{"localhost", 1138}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:-1138"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:-1138'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("0", port_str.c_str());
-  EXPECT_EQ(0, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("*:65535", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("65535", port_str.c_str());
-  EXPECT_EQ(65535, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345",
-  host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("12345"),
+   llvm::HasValue(Socket::HostAndPort{"", 12345}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:0"),
+   llvm::HasValue(Socket::HostAndPort{"*", 0}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:65535"),
+   llvm::HasValue(Socket::HostAndPort{"*", 65535}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
 }
 
 #if LLDB_ENABLE_POSIX
Index: lldb/tools/lldb-server/Acceptor.cpp
===
--- lldb/tools/lldb-server/Acceptor.cpp
+++ lldb/tools/lldb-server/Acceptor.cpp
@@ -96,8 +96,7 @@
 std::string port_str;
 uint16_t port;
 // Try to match socket name as $host:port - e.g., localhost:
-if (!llvm::errorToBool(
-Socket::DecodeHostAndPort(name, host_str, port_str, port)))
+if (!llvm::errorToBool(Socket::DecodeHostAndPort(name).takeError()))
   socket_protocol = Socket::ProtocolTcp;
   }
 
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
@@ -3917,12 +3917,8 @@
   if (url_arg.startswith(":"))
 host_p

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py:45
 "[3] = 3",
 "[6] = 6"])
 

danilashtefan wrote:
> wallace wrote:
> > can you add additional assertions like the one you did in 
> > https://reviews.llvm.org/D112180 that uses ValueCheck?
> I have made check method more generic and less verbose. Before it was 
> specifically checking the "ii" variable, that was a set of size 7. With 
> value check approach we can now test set and others. 
> 
> I guess, that I can delete all of the
> ```
> self.expect() 
> ```
> and simply call check method with the expected size. Please, let me know if I 
> should do it or leave it as it is. Thanks
yes, the code is good as you did it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

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

[Lldb-commits] [PATCH] D112632: [lldb] [Host/Terminal] Add llvm_unreachable() to appease Windows

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, teemperor, krytarowski, thakis.
mgorny requested review of this revision.

https://reviews.llvm.org/D112632

Files:
  lldb/source/Host/common/Terminal.cpp


Index: lldb/source/Host/common/Terminal.cpp
===
--- lldb/source/Host/common/Terminal.cpp
+++ lldb/source/Host/common/Terminal.cpp
@@ -77,6 +77,8 @@
   if (enabled)
 fd_termios.c_lflag |= ECHO;
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
@@ -91,6 +93,8 @@
   if (enabled)
 fd_termios.c_lflag |= ICANON;
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
@@ -109,6 +113,8 @@
   fd_termios.c_cc[VTIME] = 0;
 
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -278,6 +284,8 @@
 std::error_code(errno, std::generic_category()),
 "setting output baud rate failed");
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -301,6 +309,8 @@
 "invalid stop bit count: %d (must be 1 or 2)", stop_bits);
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -332,6 +342,8 @@
 }
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -352,6 +364,8 @@
   fd_termios.c_iflag |= PARMRK;
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -374,6 +388,8 @@
 "hardware flow control is not supported by the platform");
   return llvm::Error::success();
 #endif // defined(CRTSCTS)
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 


Index: lldb/source/Host/common/Terminal.cpp
===
--- lldb/source/Host/common/Terminal.cpp
+++ lldb/source/Host/common/Terminal.cpp
@@ -77,6 +77,8 @@
   if (enabled)
 fd_termios.c_lflag |= ECHO;
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
@@ -91,6 +93,8 @@
   if (enabled)
 fd_termios.c_lflag |= ICANON;
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
@@ -109,6 +113,8 @@
   fd_termios.c_cc[VTIME] = 0;
 
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -278,6 +284,8 @@
 std::error_code(errno, std::generic_category()),
 "setting output baud rate failed");
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -301,6 +309,8 @@
 "invalid stop bit count: %d (must be 1 or 2)", stop_bits);
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -332,6 +342,8 @@
 }
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -352,6 +364,8 @@
   fd_termios.c_iflag |= PARMRK;
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
@@ -374,6 +388,8 @@
 "hardware flow control is not supported by the platform");
   return llvm::Error::success();
 #endif // defined(CRTSCTS)
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112495: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.
Herald added a subscriber: JDevlieghere.

This patch broke `TestPlatformSDK.py` on GreenDragon 
(https://green.lab.llvm.org/green/job/lldb-cmake/37156/)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112495

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

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan updated this revision to Diff 382729.
danilashtefan added a comment.

Refactoring


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-#include 
-#include 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set &ref, std::set *ptr)
-{
-// Stop here to check by ref and ptr
-return;
-} 
-
-int main()
-{
-std::set ii;
-thefoo_rw(1);  // Set break point at this line.
-	
-	ii.insert(0);
-	ii.insert(1);
-	ii.insert(2);
-	ii.insert(3);
-	ii.insert(4);
-	ii.insert(5);
-thefoo_rw(1);  // Set break point at this line.
-
-	ii.insert(6);
-	thefoo_rw(1);  // Set break point at this line.
-
-by_ref_and_ptr(ii, &ii);
-
-	ii.clear();
-	thefoo_rw(1);  // Set break point at this line.
-
-	std::set ss;
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("a");
-	ss.insert("a very long string is right here");
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("b");
-	ss.insert("c");
-	thefoo_rw(1);  // Set break point at this line.
-	
-	ss.erase("b");
-	thefoo_rw(1);  // Set break point at this line.
-
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
@@ -0,0 +1,55 @@
+#include 
+#include 
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+arg = 0;
+  if (!arg)
+arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set &ref, std::set *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
@@ -9,6 +9,8 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
@@ -18,34 +20,29 @@
 TestBase.setUp(self)
 self.namespace = 'std'
 
-def getVariableType(self, name):
+def findVariable(self, name):
 var = self.frame().FindVariable(name)
 self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
 return var.GetType().GetDisplayTypeName()
 
-def check_ii(self, var_name):
-""" This checks the value of the bitset stored in ii at the call to by_ref_and_ptr.
-We use this to make sure we get the same values for ii when we look at the object
-directly, and when we look at a reference to the object. """
-self.expect(
-"frame variable " + var_name,
-substrs=["size=7",
-   

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[walter erquinigo via Phabricator via lldb-commits]

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

Just fix that last thing and I'll land your patch :) Thanks




Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:318
 
-class StdMapSynthProvider:
+class StdSetOrMapSynthProvider:
 

wallace wrote:
> Make a comment above like
> 
>   '''
>   Set and Map have the same underlying data structure, therefore we can use 
> exactly the same implementation for the formatter.
>   '''
actual python documentation uses ''' for comments, not #


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

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

[Lldb-commits] [PATCH] D112564: [lldb] Add support for custom char8_t types with different name

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 added a comment.

In D112564#3089399 , @labath wrote:

> What exactly do you need the test to do? Is printing a global variable 
> (`char8_t_ish foo[] = "my string"`) sufficient?
> If so, you can take a look at the tests in `test/Shell/SymbolFile/DWARF` for 
> inspiration. There are .s and .ll tests there.
> The pattern would roughly be:
>
>   # RUN: llvm-mc (or llc) -o %t %s
>   # RUN: %lldb %t -o "target variable foo" | FileCheck %s
>   
>   # CHECK: (char8_t_ish[]) foo = "my string"
>   
>   your code here

Yeah, it is a bit more complicated than that although, due to 
https://bugs.llvm.org/show_bug.cgi?id=45856 . I'm going to take a look at those 
tests and write some.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112564

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

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan updated this revision to Diff 382734.
danilashtefan added a comment.

Last thing corrected :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-#include 
-#include 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set &ref, std::set *ptr)
-{
-// Stop here to check by ref and ptr
-return;
-} 
-
-int main()
-{
-std::set ii;
-thefoo_rw(1);  // Set break point at this line.
-	
-	ii.insert(0);
-	ii.insert(1);
-	ii.insert(2);
-	ii.insert(3);
-	ii.insert(4);
-	ii.insert(5);
-thefoo_rw(1);  // Set break point at this line.
-
-	ii.insert(6);
-	thefoo_rw(1);  // Set break point at this line.
-
-by_ref_and_ptr(ii, &ii);
-
-	ii.clear();
-	thefoo_rw(1);  // Set break point at this line.
-
-	std::set ss;
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("a");
-	ss.insert("a very long string is right here");
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("b");
-	ss.insert("c");
-	thefoo_rw(1);  // Set break point at this line.
-	
-	ss.erase("b");
-	thefoo_rw(1);  // Set break point at this line.
-
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
@@ -0,0 +1,55 @@
+#include 
+#include 
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+arg = 0;
+  if (!arg)
+arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set &ref, std::set *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
@@ -9,6 +9,8 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
@@ -18,34 +20,29 @@
 TestBase.setUp(self)
 self.namespace = 'std'
 
-def getVariableType(self, name):
+def findVariable(self, name):
 var = self.frame().FindVariable(name)
 self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
 return var.GetType().GetDisplayTypeName()
 
-def check_ii(self, var_name):
-""" This checks the value of the bitset stored in ii at the call to by_ref_and_ptr.
-We use this to make sure we get the same values for ii when we look at the object
-directly, and when we look at a reference to the object. """
-self.expect(
-"frame variable " + var_name,
-substrs=["s

[Lldb-commits] [lldb] 3eb9e65 - [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[Walter Erquinigo via lldb-commits]

Author: Danil Stefaniuc
Date: 2021-10-27T11:55:11-07:00
New Revision: 3eb9e6536a6a40f116655240b264292efabdc928

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

LOG: [formatters] Add a libstdcpp formatter for set and unify tests across 
stdlibs

This diff adds a data formatter for libstdcpp's set. Besides, it unifies the 
tests for set for libcxx and libstdcpp for maintainability.

Reviewed By: wallace

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

Added: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp

Modified: 
lldb/examples/synthetic/gnu_libstdcpp.py
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Removed: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp



diff  --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index 528b28349bb1a..21c89754c0825 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -314,19 +314,23 @@ def update(self):
 def has_children(self):
 return True
 
-
-class StdMapSynthProvider:
+"""
+Set and Map have the same underlying data structure,
+therefore we can use exactly the same implementation for the formatter.
+"""
+class StdSetOrMapSynthProvider:
 
 def __init__(self, valobj, dict):
 logger = lldb.formatters.Logger.Logger()
 self.valobj = valobj
 self.count = None
-logger >> "Providing synthetic children for a map named " + \
+self.kind = "set" if "set" in valobj.GetTypeName() else "map"
+logger >> "Providing synthetic children for a " + self.kind + " named 
" + \
 str(valobj.GetName())
 
 # we need this function as a temporary workaround for 
rdar://problem/10801549
 # which prevents us from extracting the std::pair SBType out of the 
template
-# arguments for _Rep_Type _M_t in the map itself - because we have to make 
up the
+# arguments for _Rep_Type _M_t in the object itself - because we have to 
make up the
 # typename and then find it, we may hit the situation were std::string has 
multiple
 # names but only one is actually referenced in the debug information. 
hence, we need
 # to replace the longer versions of std::string with the shorter one in 
order to be able
@@ -349,7 +353,7 @@ def update(self):
 # later
 self.count = None
 try:
-# we will set this to True if we find out that discovering a node 
in the map takes more steps than the overall size of the RB tree
+# we will set this to True if we find out that discovering a node 
in the object takes more steps than the overall size of the RB tree
 # if this gets set to True, then we will merrily return None for
 # any child from that moment on
 self.garbage = False

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 03e3e46523085..875706edd77eb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -902,14 +902,19 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   RegularExpression("^std::map<.+> >(( )?&)?$"),
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
-  "lldb.formatters.cpp.gnu_libstdcpp.StdMapSynthProvider")));
+  "lldb.formatters.cpp.gnu_libstdcpp.StdSetOrMapSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  RegularExpression("^std::set<.+> >(( )?&)?$"),
+  SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  stl_deref_flags,
+  "lldb.formatters.cpp.gnu_libstdcpp.StdSetOrMapSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
   RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
-  stl_summary_flags.SetSkipPointers(true);
+  stl_summary_flags.SetSkipPointers(false);
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(

[Lldb-commits] [PATCH] D112537: [formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[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 rG3eb9e6536a6a: [formatters] Add a libstdcpp formatter for set 
and unify tests across stdlibs (authored by danilashtefan, committed by Walter 
Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D112537?vs=382734&id=382742#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112537

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-#include 
-#include 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set &ref, std::set *ptr)
-{
-// Stop here to check by ref and ptr
-return;
-} 
-
-int main()
-{
-std::set ii;
-thefoo_rw(1);  // Set break point at this line.
-	
-	ii.insert(0);
-	ii.insert(1);
-	ii.insert(2);
-	ii.insert(3);
-	ii.insert(4);
-	ii.insert(5);
-thefoo_rw(1);  // Set break point at this line.
-
-	ii.insert(6);
-	thefoo_rw(1);  // Set break point at this line.
-
-by_ref_and_ptr(ii, &ii);
-
-	ii.clear();
-	thefoo_rw(1);  // Set break point at this line.
-
-	std::set ss;
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("a");
-	ss.insert("a very long string is right here");
-	thefoo_rw(1);  // Set break point at this line.
-
-	ss.insert("b");
-	ss.insert("c");
-	thefoo_rw(1);  // Set break point at this line.
-	
-	ss.erase("b");
-	thefoo_rw(1);  // Set break point at this line.
-
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/main.cpp
@@ -0,0 +1,55 @@
+#include 
+#include 
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+arg = 0;
+  if (!arg)
+arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set &ref, std::set *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
@@ -9,6 +9,8 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
@@ -18,34 +20,29 @@
 TestBase.setUp(self)
 self.namespace = 'std'
 
-def getVariableType(self, name):
+def findVariable(self, name):
 var = self.frame().FindVariable(name)
 self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
 return var.GetType().GetDisplayTypeName()
 
-def check_ii(self, var_name):
-""" This checks the value of the bitset stored

[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 created this revision.
ljmf00 added projects: LLDB, LLVM.
Herald added a subscriber: JDevlieghere.
ljmf00 requested review of this revision.
Herald added a subscriber: lldb-commits.

  Signed-off-by: Luís Ferreira 

This patch refactors C/C++ formatters to avoid repetitive code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112658

Files:
  lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

Index: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
@@ -32,79 +32,86 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+namespace {
+
+  template
+  static bool CharStringSummaryProvider(ValueObject &valobj, Stream &stream) {
+ProcessSP process_sp = valobj.GetProcessSP();
+if (!process_sp)
+  return false;
+
+lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
+if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
+  return false;
+
+StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
+options.SetLocation(valobj_addr);
+options.SetProcessSP(process_sp);
+options.SetStream(&stream);
+if (ElemType == StringPrinter::StringElementType::UTF8)
+  options.SetPrefixToken("u8");
+else if (ElemType == StringPrinter::StringElementType::UTF16)
+  options.SetPrefixToken("u");
+else if (ElemType == StringPrinter::StringElementType::UTF32)
+  options.SetPrefixToken("U");
+
+if (!StringPrinter::ReadStringAndDumpToStream(options))
+  stream.Printf("Summary Unavailable");
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
-return false;
-
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream(&stream);
-  options.SetPrefixToken("u8");
-
-  if (!StringPrinter::ReadStringAndDumpToStream<
-  StringPrinter::StringElementType::UTF8>(options)) {
-stream.Printf("Summary Unavailable");
 return true;
   }
 
-  return true;
-}
-
-bool lldb_private::formatters::Char16StringSummaryProvider(
-ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+  template
+  static bool CharSummaryProvider(ValueObject &valobj, Stream &stream) {
+DataExtractor data;
+Status error;
+valobj.GetData(data, error);
+
+if (error.Fail())
+  return false;
+
+std::string value;
+StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
+
+if (ElemType == StringPrinter::StringElementType::UTF8) {
+  options.SetPrefixToken("u8");
+  valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF16) {
+  options.SetPrefixToken("u");
+  valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF32) {
+  options.SetPrefixToken("U");
+  valobj.GetValueAsCString(lldb::eFormatUnicode32, value);
+}
+
+if (!value.empty())
+  stream.Printf("%s ", value.c_str());
+
+options.SetData(std::move(data));
+options.SetStream(&stream);
+options.SetQuote('\'');
+options.SetSourceSize(1);
+options.SetBinaryZeroIsTerminator(false);
+
+return StringPrinter::ReadBufferAndDumpToStream(options);
+  }
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
-return false;
+} // namespace
 
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream(&stream);
-  options.SetPrefixToken("u");
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-  StringPrinter::StringElementType::UTF16>(options)) {
-stream.Printf("Summary Unavailable");
-return true;
-  }
+bool lldb_private::formatters::Char8StringSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider(valobj, stream);
+}
 
-  return true;
+bool lldb_private::formatters::Char16StringSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider(valobj, stream);
 }
 
 bool lldb_private::formatters::Char32StringSummaryProvider(
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_s

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, jasonmolenda.
JDevlieghere requested review of this revision.

Upstream the calls to ABI::FixCodeAddress in the Objective-C language
runtime.


https://reviews.llvm.org/D112662

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UtilityFunction.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
@@ -134,6 +135,10 @@
   target_addr_value);
 m_impl_function->DeallocateFunctionResults(exc_ctx, m_args_addr);
 lldb::addr_t target_addr = target_addr_value.GetScalar().ULongLong();
+
+if (ABISP abi_sp = GetThread().GetProcess()->GetABI()) {
+  target_addr = abi_sp->FixCodeAddress(target_addr);
+}
 Address target_so_addr;
 target_so_addr.SetOpcodeLoadAddress(target_addr, exc_ctx.GetTargetPtr());
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1445,13 +1445,21 @@
 return objc_class_sp;
 
   objc_class_sp = GetClassDescriptorFromISA(isa);
-  if (isa && !objc_class_sp) {
-Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_TYPES));
-LLDB_LOGF(log,
-  "0x%" PRIx64 ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was "
-  "not in class descriptor cache 0x%" PRIx64,
-  isa_pointer, isa);
+  if (!objc_class_sp) {
+if (ABISP abi_sp = process->GetABI()) {
+  isa = abi_sp->FixCodeAddress(isa);
+  objc_class_sp = GetClassDescriptorFromISA(isa);
+}
   }
+
+  if (objc_class_sp)
+return objc_class_sp;
+
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_TYPES));
+  LLDB_LOGF(log,
+"0x%" PRIx64 ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was "
+"not in class descriptor cache 0x%" PRIx64,
+isa_pointer, isa);
   return objc_class_sp;
 }
 
@@ -2695,6 +2703,13 @@
   return nullptr;
 actual_class_descriptor_sp =
 m_runtime.GetClassDescriptorFromISA((ObjCISA)slot_data);
+if (!actual_class_descriptor_sp) {
+  if (ABISP abi_sp = process->GetABI()) {
+ObjCISA fixed_isa = abi_sp->FixCodeAddress((ObjCISA)slot_data);
+actual_class_descriptor_sp =
+m_runtime.GetClassDescriptorFromISA(fixed_isa);
+  }
+}
 if (!actual_class_descriptor_sp)
   return ObjCLanguageRuntime::ClassDescriptorSP();
 m_cache[slot] = actual_class_descriptor_sp;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -9,6 +9,7 @@
 #include "AppleObjCClassDescriptorV2.h"
 
 #include "lldb/Expression/FunctionCaller.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
@@ -73,6 +74,10 @@
   m_flags = (uint8_t)(data_NEVER_USE & (lldb::addr_t)3);
   m_data_ptr = data_NEVER_USE & GetClassDataMask(process);
 
+  if (ABISP abi_sp = process->GetABI()) {
+m_isa = abi_sp->FixCodeAddress(m_isa);
+m_superclass = abi_sp->FixCodeAddress(m_superclass);
+  }
   return true;
 }
 
@@ -105,6 +110,8 @@
   m_flags = extractor.GetU32_unchecked(&cursor);
   m_version = extractor.GetU32_unchecked(&cursor);
   m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+  if (ABISP abi_sp = process->GetABI())
+m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   m_method_list_ptr = extractor.GetAddress_unchecked(&cursor);
   m_properties_ptr = extractor.GetAddress_unchecked(&cursor);
   m_firstSubclass = extractor.GetAddress_unchecked(&cursor);
@@ -120,6 +127,8 @@
 process->GetByteOrder(),
  

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

[Adrian Prantl via Phabricator via lldb-commits]

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

This is all good of course, but I wonder if we should lift this into the type 
system and have functions returned a UnstrippedAddress type that you can only 
turn into an addr_t by calling FixCodeAddress.




Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2705
 actual_class_descriptor_sp =
 m_runtime.GetClassDescriptorFromISA((ObjCISA)slot_data);
+if (!actual_class_descriptor_sp) {

Why not sink this into m_runtime.GetClassDescriptorFromISA?


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

https://reviews.llvm.org/D112662

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

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 382795.
JDevlieghere marked an inline comment as done.

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

https://reviews.llvm.org/D112662

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
@@ -273,10 +274,17 @@
 ObjCLanguageRuntime::GetClassDescriptorFromISA(ObjCISA isa) {
   if (isa) {
 UpdateISAToDescriptorMap();
+
 ObjCLanguageRuntime::ISAToDescriptorIterator pos =
 m_isa_to_descriptor.find(isa);
 if (pos != m_isa_to_descriptor.end())
   return pos->second;
+
+if (ABISP abi_sp = m_process->GetABI()) {
+  pos = m_isa_to_descriptor.find(abi_sp->FixCodeAddress(isa));
+  if (pos != m_isa_to_descriptor.end())
+return pos->second;
+}
   }
   return ClassDescriptorSP();
 }
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UtilityFunction.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
@@ -134,6 +135,10 @@
   target_addr_value);
 m_impl_function->DeallocateFunctionResults(exc_ctx, m_args_addr);
 lldb::addr_t target_addr = target_addr_value.GetScalar().ULongLong();
+
+if (ABISP abi_sp = GetThread().GetProcess()->GetABI()) {
+  target_addr = abi_sp->FixCodeAddress(target_addr);
+}
 Address target_so_addr;
 target_so_addr.SetOpcodeLoadAddress(target_addr, exc_ctx.GetTargetPtr());
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -9,6 +9,7 @@
 #include "AppleObjCClassDescriptorV2.h"
 
 #include "lldb/Expression/FunctionCaller.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
@@ -73,6 +74,10 @@
   m_flags = (uint8_t)(data_NEVER_USE & (lldb::addr_t)3);
   m_data_ptr = data_NEVER_USE & GetClassDataMask(process);
 
+  if (ABISP abi_sp = process->GetABI()) {
+m_isa = abi_sp->FixCodeAddress(m_isa);
+m_superclass = abi_sp->FixCodeAddress(m_superclass);
+  }
   return true;
 }
 
@@ -105,6 +110,8 @@
   m_flags = extractor.GetU32_unchecked(&cursor);
   m_version = extractor.GetU32_unchecked(&cursor);
   m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+  if (ABISP abi_sp = process->GetABI())
+m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   m_method_list_ptr = extractor.GetAddress_unchecked(&cursor);
   m_properties_ptr = extractor.GetAddress_unchecked(&cursor);
   m_firstSubclass = extractor.GetAddress_unchecked(&cursor);
@@ -120,6 +127,8 @@
 process->GetByteOrder(),
 process->GetAddressByteSize());
 m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+if (ABISP abi_sp = process->GetABI())
+  m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   }
 
   return true;
@@ -231,6 +240,8 @@
   DataBufferHeap buffer(size, '\0');
   Status error;
 
+  if (ABISP abi_sp = process->GetABI())
+addr = abi_sp->FixCodeAddress(addr);
   process->ReadMemory(addr, buffer.GetBytes(), size, error);
   if (error.Fail()) {
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 382803.
clayborg added a comment.

Changes in this version:

- If we fail to get "details" from the structured data, we put the error string 
into the "details" dictionary with the error string.
- Added "numLocations" (int64_t) and "numResolvedLocations" (int64_t) and 
"internal" (bool) into each breakpoint's stats.
- Include the internal breakpoints in the "breakpoints" so we are tracking all 
breakpoints that are being set.
- Removed the top level "names" field because it is already contained in the 
"details".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -283,3 +283,85 @@
 ]
 self.assertNotEqual(exe_module, None)
 self.verify_keys(exe_module, 'module dict for "%s"' % (exe), module_keys)
+
+def test_breakpoints(self):
+"""Test "statistics dump"
+
+Output expected to be something like:
+
+{
+  "modules" : [...],
+  "targets" : [
+{
+"firstStopTime": 0.3416449280001,
+"launchOrAttachTime": 0.3196960540001,
+"moduleIdentifiers": [...],
+"targetCreateTime": 0.00408630398
+"expressionEvaluation": {
+"failures": 0,
+"successes": 0
+},
+"frameVariable": {
+"failures": 0,
+"successes": 0
+},
+"breakpoints": [
+{
+"details": {...},
+"id": 1,
+"resolveTime": 2.65438675
+},
+{
+"details": {...},
+"id": 2,
+"resolveTime": 4.363258166997
+}
+]
+}
+],
+"totalDebugInfoByteSize": 182522234,
+"totalDebugInfoIndexTime": 2.33343,
+"totalDebugInfoParseTime": 8.212140024071,
+"totalSymbolTableParseTime": 0.123,
+"totalSymbolTableIndexTime": 0.234,
+"totalBreakpointResolveTime": 7.017644917001
+}
+
+"""
+target = self.createTestTarget()
+self.runCmd("b main.cpp:7")
+self.runCmd("b a_function")
+debug_stats = self.get_stats()
+debug_stat_keys = [
+'modules', 
+'targets',
+'totalSymbolTableParseTime',
+'totalSymbolTableIndexTime',
+'totalDebugInfoParseTime',
+'totalDebugInfoIndexTime',
+'totalDebugInfoByteSize',
+]
+self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+target_stats = debug_stats['targets'][0]
+keys_exist = [
+'breakpoints',
+'expressionEvaluation',
+'frameVariable',
+'targetCreateTime',
+'moduleIdentifiers',
+'totalBreakpointResolveTime',
+]
+self.verify_keys(target_stats, '"stats"', keys_exist, None)
+self.assertGreater(target_stats['totalBreakpointResolveTime'], 0.0)
+breakpoints = target_stats['breakpoints']
+bp_keys_exist = [
+'details',
+'id',
+'internal',
+'numLocations',
+'numResolvedLocations',
+'resolveTime'
+]
+for breakpoint in breakpoints:
+self.verify_keys(breakpoint, 'target_stats["breakpoints"]', 
+ bp_keys_exist, None)
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -80,6 +80,25 @@
   }
   target_metrics_json.try_emplace("targetCreateTime", m_create_time.count());
 
+  json::Array breakpoints_array;
+  double totalBreakpointResolveTime = 0.0;
+  // Rport both the normal breakpoint list and the internal breakpoint list.
+  for (int i = 0; i < 2; ++i) {
+BreakpointList &breakpoints = target.GetBreakpointList(i == 1);
+std::unique_lock lock;
+breakpoints.GetListMutex(lock);
+size_t num_breakpoints = breakpoints.GetSize();
+for (size_t i = 0; i < num_breakpoints; i++) {
+  Breakpoint 

[Lldb-commits] [lldb] 8bac9e3 - [lldb] Fixup code addresses in the Objective-C language runtime

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-10-27T14:48:03-07:00
New Revision: 8bac9e3686e05916db88dab29b3c7fee70090d81

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

LOG: [lldb] Fixup code addresses in the Objective-C language runtime

Upstream the calls to ABI::FixCodeAddress in the Objective-C language
runtime.

Differential revision: https://reviews.llvm.org/D112662

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index 405b8a6f16b7..162ccad3cdcd 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -9,6 +9,7 @@
 #include "AppleObjCClassDescriptorV2.h"
 
 #include "lldb/Expression/FunctionCaller.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
@@ -73,6 +74,10 @@ bool ClassDescriptorV2::objc_class_t::Read(Process *process,
   m_flags = (uint8_t)(data_NEVER_USE & (lldb::addr_t)3);
   m_data_ptr = data_NEVER_USE & GetClassDataMask(process);
 
+  if (ABISP abi_sp = process->GetABI()) {
+m_isa = abi_sp->FixCodeAddress(m_isa);
+m_superclass = abi_sp->FixCodeAddress(m_superclass);
+  }
   return true;
 }
 
@@ -105,6 +110,8 @@ bool ClassDescriptorV2::class_rw_t::Read(Process *process, 
lldb::addr_t addr) {
   m_flags = extractor.GetU32_unchecked(&cursor);
   m_version = extractor.GetU32_unchecked(&cursor);
   m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+  if (ABISP abi_sp = process->GetABI())
+m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   m_method_list_ptr = extractor.GetAddress_unchecked(&cursor);
   m_properties_ptr = extractor.GetAddress_unchecked(&cursor);
   m_firstSubclass = extractor.GetAddress_unchecked(&cursor);
@@ -120,6 +127,8 @@ bool ClassDescriptorV2::class_rw_t::Read(Process *process, 
lldb::addr_t addr) {
 process->GetByteOrder(),
 process->GetAddressByteSize());
 m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+if (ABISP abi_sp = process->GetABI())
+  m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   }
 
   return true;
@@ -231,6 +240,8 @@ bool ClassDescriptorV2::method_list_t::Read(Process 
*process,
   DataBufferHeap buffer(size, '\0');
   Status error;
 
+  if (ABISP abi_sp = process->GetABI())
+addr = abi_sp->FixCodeAddress(addr);
   process->ReadMemory(addr, buffer.GetBytes(), size, error);
   if (error.Fail()) {
 return false;

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
index 1dc8034c537a..f935ae7db8c0 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UtilityFunction.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
@@ -134,6 +135,10 @@ bool 
AppleThreadPlanStepThroughObjCTrampoline::ShouldStop(Event *event_ptr) {
   target_addr_value);
 m_impl_function->DeallocateFunctionResults(exc_ctx, m_args_addr);
 lldb::addr_t target_addr = target_addr_value.GetScalar().ULongLong();
+
+if (ABISP abi_sp = GetThread().GetProcess()->GetABI()) {
+  target_addr = abi_sp->FixCodeAddress(target_addr);
+}
 Address target_so_addr;
 target_so_addr.SetOpcodeLoadAddress(target_addr, exc_ctx.GetTargetPtr());
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index 65bf3e6af626..0cc96e43e195 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Target/Targ

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bac9e3686e0: [lldb] Fixup code addresses in the Objective-C 
language runtime (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112662

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
@@ -273,10 +274,17 @@
 ObjCLanguageRuntime::GetClassDescriptorFromISA(ObjCISA isa) {
   if (isa) {
 UpdateISAToDescriptorMap();
+
 ObjCLanguageRuntime::ISAToDescriptorIterator pos =
 m_isa_to_descriptor.find(isa);
 if (pos != m_isa_to_descriptor.end())
   return pos->second;
+
+if (ABISP abi_sp = m_process->GetABI()) {
+  pos = m_isa_to_descriptor.find(abi_sp->FixCodeAddress(isa));
+  if (pos != m_isa_to_descriptor.end())
+return pos->second;
+}
   }
   return ClassDescriptorSP();
 }
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UtilityFunction.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
@@ -134,6 +135,10 @@
   target_addr_value);
 m_impl_function->DeallocateFunctionResults(exc_ctx, m_args_addr);
 lldb::addr_t target_addr = target_addr_value.GetScalar().ULongLong();
+
+if (ABISP abi_sp = GetThread().GetProcess()->GetABI()) {
+  target_addr = abi_sp->FixCodeAddress(target_addr);
+}
 Address target_so_addr;
 target_so_addr.SetOpcodeLoadAddress(target_addr, exc_ctx.GetTargetPtr());
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -9,6 +9,7 @@
 #include "AppleObjCClassDescriptorV2.h"
 
 #include "lldb/Expression/FunctionCaller.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
@@ -73,6 +74,10 @@
   m_flags = (uint8_t)(data_NEVER_USE & (lldb::addr_t)3);
   m_data_ptr = data_NEVER_USE & GetClassDataMask(process);
 
+  if (ABISP abi_sp = process->GetABI()) {
+m_isa = abi_sp->FixCodeAddress(m_isa);
+m_superclass = abi_sp->FixCodeAddress(m_superclass);
+  }
   return true;
 }
 
@@ -105,6 +110,8 @@
   m_flags = extractor.GetU32_unchecked(&cursor);
   m_version = extractor.GetU32_unchecked(&cursor);
   m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+  if (ABISP abi_sp = process->GetABI())
+m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   m_method_list_ptr = extractor.GetAddress_unchecked(&cursor);
   m_properties_ptr = extractor.GetAddress_unchecked(&cursor);
   m_firstSubclass = extractor.GetAddress_unchecked(&cursor);
@@ -120,6 +127,8 @@
 process->GetByteOrder(),
 process->GetAddressByteSize());
 m_ro_ptr = extractor.GetAddress_unchecked(&cursor);
+if (ABISP abi_sp = process->GetABI())
+  m_ro_ptr = abi_sp->FixCodeAddress(m_ro_ptr);
   }
 
   return true;
@@ -231,6 +240,8 @@
   DataBufferHeap buffer(size, '\0');
   Status error;
 
+  if (ABISP abi_sp = process->GetABI())
+addr = abi_sp->FixCodeAddress(addr);
   process->ReadMemory(addr, buffer.GetBytes(), size, error);
   if (error.Fail()) {
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Breakpoint/Breakpoint.cpp:1120
+else
+  llvm::consumeError(expected_value.takeError());
+  }

wallace wrote:
> err, i'm not a fan of this. Why don't you just make this return an expected 
> and make the main command fail if the data couldn't be gathered? That will be 
> better than concealing some possible parsing issues.
How about I put the error string into the "details" as a key/value pair if this 
has an error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good to me, but the expression parser isn't my main area of expertise. I 
would like to see someone with more knowledge in the expression parser to make 
the final ok. Raphael?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [lldb] 8dbbe33 - Revert "[lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse"

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-27T23:57:33+02:00
New Revision: 8dbbe3356b0bda56c7d21e5a3b07d7bfe601bc22

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

LOG: Revert "[lldb] [Host/ConnectionFileDescriptor] Refactor to improve code 
reuse"

This reverts commit e1acadb61dfc0810656219c6314019d5132f2c61.

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 306763025d4bc..780c6e6a1 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -75,6 +75,18 @@ class Socket : public IOObject {
   static llvm::Expected>
   UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
 
+  static Status UnixDomainConnect(llvm::StringRef host_and_port,
+  bool child_processes_inherit,
+  Socket *&socket);
+  static Status UnixDomainAccept(llvm::StringRef host_and_port,
+ bool child_processes_inherit, Socket 
*&socket);
+  static Status UnixAbstractConnect(llvm::StringRef host_and_port,
+bool child_processes_inherit,
+Socket *&socket);
+  static Status UnixAbstractAccept(llvm::StringRef host_and_port,
+   bool child_processes_inherit,
+   Socket *&socket);
+
   int GetOption(int level, int option_name, int &option_value);
   int SetOption(int level, int option_name, int option_value);
 

diff  --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e91..12eab5fc8ae83 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -16,7 +16,6 @@
 #include "lldb/lldb-forward.h"
 
 #include "lldb/Host/Pipe.h"
-#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Connection.h"
 #include "lldb/Utility/IOObject.h"
 
@@ -74,18 +73,9 @@ class ConnectionFileDescriptor : public Connection {
   void CloseCommandPipe();
 
   lldb::ConnectionStatus
-  AcceptSocket(Socket::SocketProtocol socket_protocol,
-   llvm::StringRef socket_name,
-   llvm::function_ref post_listen_callback,
-   Status *error_ptr);
-
-  lldb::ConnectionStatus ConnectSocket(Socket::SocketProtocol socket_protocol,
-   llvm::StringRef socket_name,
-   Status *error_ptr);
-
-  lldb::ConnectionStatus AcceptTCP(llvm::StringRef host_and_port,
-   socket_id_callback_type socket_id_callback,
-   Status *error_ptr);
+  SocketListenAndAccept(llvm::StringRef host_and_port,
+socket_id_callback_type socket_id_callback,
+Status *error_ptr);
 
   lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
 socket_id_callback_type socket_id_callback,
@@ -96,24 +86,24 @@ class ConnectionFileDescriptor : public Connection {
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  ConnectNamedSocket(llvm::StringRef socket_name,
+  NamedSocketConnect(llvm::StringRef socket_name,
  socket_id_callback_type socket_id_callback,
  Status *error_ptr);
 
   lldb::ConnectionStatus
-  AcceptNamedSocket(llvm::StringRef socket_name,
+  NamedSocketAccept(llvm::StringRef socket_name,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  AcceptAbstractSocket(llvm::StringRef socket_name,
-   socket_id_callback_type socket_id_callback,
-   Status *error_ptr);
+  UnixAbstractSocketAccept(llvm::StringRef socket_name,
+   socket_id_callback_type socket_id_callback,
+   Status *error_ptr);
 
   lldb::ConnectionStatus
-  ConnectAbstractSocket(llvm::StringRef socket_name,
-socket_id_callback_type socket_id_callback,
-Status *error_ptr);
+  UnixAbstractSocketConnect(llvm::StringRef socket_name,
+socket_id_callback_type socket_id_callback,
+Status *error_ptr);
 
   lldb::ConnectionStatus ConnectFD(llvm::StringRef args,
socket_id_callbac

[Lldb-commits] [PATCH] D112495: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Looking at the test backtrace, it seems that we couldn't connect to debugserver:

  File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py",
 line 113, in test_macos_sdk
self.assertTrue(connected, "could not connect to debugserver")

I checked the `SBCommandReturnObject.GetError` output, which says `Failed to 
connect port`. The only occurence for this error is in :

  lldb/source/Host/common/TCPSocket.cpp
  183:  error.SetErrorString("error: Failed to connect port");

I reverted the patch to fix GreenDragon, I'll try to investigate more tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112495

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

[Lldb-commits] [PATCH] D112495: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Build run #37166 (https://green.lab.llvm.org/green/job/lldb-cmake/37166/) 
confirms that this patch caused the failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112495

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

[Lldb-commits] [lldb] a0c1e75 - [lldb] Skip TestCCallingConventions.test_ms_abi on arm64

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-10-27T16:08:14-07:00
New Revision: a0c1e7571fe4f61d0f976bff3cdc50d1c2a578a6

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

LOG: [lldb] Skip TestCCallingConventions.test_ms_abi on arm64

rdar://84528755

Added: 


Modified: 
lldb/test/API/lang/c/calling-conventions/TestCCallingConventions.py

Removed: 




diff  --git 
a/lldb/test/API/lang/c/calling-conventions/TestCCallingConventions.py 
b/lldb/test/API/lang/c/calling-conventions/TestCCallingConventions.py
index 5156190bc071..9a5acd5a1887 100644
--- a/lldb/test/API/lang/c/calling-conventions/TestCCallingConventions.py
+++ b/lldb/test/API/lang/c/calling-conventions/TestCCallingConventions.py
@@ -47,6 +47,7 @@ def test_regcall(self):
 return
 self.expect_expr("func(1, 2, 3, 4)", result_type="int", 
result_value="10")
 
+@expectedFailureDarwin(archs=["arm64", "arm64e"]) # rdar://84528755
 def test_ms_abi(self):
 if not self.build_and_run("ms_abi.c"):
 return



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

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
JDevlieghere requested review of this revision.

Create a valid triple in the Darwin builder. Currently it was
incorrectly treating the os and version as two separate components in
the triple.


https://reviews.llvm.org/D112676

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


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -57,7 +57,7 @@
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
+components = [arch, vendor, os + version, env]
 if None in components:
 return None
 return '-'.join(components)
@@ -97,7 +97,7 @@
 return []
 
 # Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
+triple = '-'.join([arch, vendor, os + version, env])
 
 # Construct min version argument
 version_min = ""


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -57,7 +57,7 @@
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
+components = [arch, vendor, os + version, env]
 if None in components:
 return None
 return '-'.join(components)
@@ -97,7 +97,7 @@
 return []
 
 # Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
+triple = '-'.join([arch, vendor, os + version, env])
 
 # Construct min version argument
 version_min = ""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Adrian Prantl via Phabricator via lldb-commits]

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

Yup that's wrong. Personally I would only append env to the list if non-empty. 
Thanks!


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

https://reviews.llvm.org/D112676

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

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 382851.
JDevlieghere added a comment.

Skip the environment if it's empty and avoid a TypeError when concatenating 
'NoneType' and 'str'.


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

https://reviews.llvm.org/D112676

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


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -54,13 +54,20 @@
 return vendor, os, version, env
 
 
+def get_triple_str(arch, vendor, os, version, env):
+if None in [arch, vendor, os, version, env]:
+return None
+
+component = [arch, vendor, os + version]
+if env:
+components.append(env)
+return '-'.join(component)
+
+
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
-if None in components:
-return None
-return '-'.join(components)
+return get_triple_str(arch, vendor, os, version, env)
 
 def getExtraMakeArgs(self):
 """
@@ -93,12 +100,10 @@
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if vendor is None or os is None or version is None or env is None:
+triple = get_triple_str(arch, vendor, os, version, env)
+if not triple:
 return []
 
-# Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
-
 # Construct min version argument
 version_min = ""
 if env == "simulator":


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -54,13 +54,20 @@
 return vendor, os, version, env
 
 
+def get_triple_str(arch, vendor, os, version, env):
+if None in [arch, vendor, os, version, env]:
+return None
+
+component = [arch, vendor, os + version]
+if env:
+components.append(env)
+return '-'.join(component)
+
+
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
-if None in components:
-return None
-return '-'.join(components)
+return get_triple_str(arch, vendor, os, version, env)
 
 def getExtraMakeArgs(self):
 """
@@ -93,12 +100,10 @@
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if vendor is None or os is None or version is None or env is None:
+triple = get_triple_str(arch, vendor, os, version, env)
+if not triple:
 return []
 
-# Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
-
 # Construct min version argument
 version_min = ""
 if env == "simulator":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[walter erquinigo via Phabricator via lldb-commits]

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

sounds good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

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

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

thanks!




Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:62
+component = [arch, vendor, os + version]
+if env:
+components.append(env)

So this tests for the empty string?


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

https://reviews.llvm.org/D112676

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

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:62
+component = [arch, vendor, os + version]
+if env:
+components.append(env)

aprantl wrote:
> So this tests for the empty string?
It tests for both, but None is already caught earlier. 


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

https://reviews.llvm.org/D112676

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

[Lldb-commits] [lldb] 2c35073 - [lldb] The os and version are not separate components in the triple

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-10-27T16:40:20-07:00
New Revision: 2c350730ca8b75727188139f033a7bd60d9383c7

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

LOG: [lldb] The os and version are not separate components in the triple

Create a valid triple in the Darwin builder. Currently it was
incorrectly treating the os and version as two separate components in
the triple.

Differential revision: https://reviews.llvm.org/D112676

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py 
b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index e182f2d70b02..6e5ba391a7b7 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -54,13 +54,20 @@ def get_triple():
 return vendor, os, version, env
 
 
+def get_triple_str(arch, vendor, os, version, env):
+if None in [arch, vendor, os, version, env]:
+return None
+
+component = [arch, vendor, os + version]
+if env:
+components.append(env)
+return '-'.join(component)
+
+
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
-if None in components:
-return None
-return '-'.join(components)
+return get_triple_str(arch, vendor, os, version, env)
 
 def getExtraMakeArgs(self):
 """
@@ -93,12 +100,10 @@ def getArchCFlags(self, arch):
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if vendor is None or os is None or version is None or env is None:
+triple = get_triple_str(arch, vendor, os, version, env)
+if not triple:
 return []
 
-# Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
-
 # Construct min version argument
 version_min = ""
 if env == "simulator":



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

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rG2c350730ca8b: [lldb] The os and version are not separate 
components in the triple (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112676

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


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -54,13 +54,20 @@
 return vendor, os, version, env
 
 
+def get_triple_str(arch, vendor, os, version, env):
+if None in [arch, vendor, os, version, env]:
+return None
+
+component = [arch, vendor, os + version]
+if env:
+components.append(env)
+return '-'.join(component)
+
+
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
-if None in components:
-return None
-return '-'.join(components)
+return get_triple_str(arch, vendor, os, version, env)
 
 def getExtraMakeArgs(self):
 """
@@ -93,12 +100,10 @@
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if vendor is None or os is None or version is None or env is None:
+triple = get_triple_str(arch, vendor, os, version, env)
+if not triple:
 return []
 
-# Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
-
 # Construct min version argument
 version_min = ""
 if env == "simulator":


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -54,13 +54,20 @@
 return vendor, os, version, env
 
 
+def get_triple_str(arch, vendor, os, version, env):
+if None in [arch, vendor, os, version, env]:
+return None
+
+component = [arch, vendor, os + version]
+if env:
+components.append(env)
+return '-'.join(component)
+
+
 class BuilderDarwin(Builder):
 def getTriple(self, arch):
 vendor, os, version, env = get_triple()
-components = [arch, vendor, os, version, env]
-if None in components:
-return None
-return '-'.join(components)
+return get_triple_str(arch, vendor, os, version, env)
 
 def getExtraMakeArgs(self):
 """
@@ -93,12 +100,10 @@
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if vendor is None or os is None or version is None or env is None:
+triple = get_triple_str(arch, vendor, os, version, env)
+if not triple:
 return []
 
-# Construct the triple from its components.
-triple = '-'.join([arch, vendor, os, version, env])
-
 # Construct min version argument
 version_min = ""
 if env == "simulator":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fb25496 - Add breakpoint resolving stats to each target.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2021-10-27T16:50:11-07:00
New Revision: fb2549683260b137f1bd62a5145ddff6d2403e64

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

LOG: Add breakpoint resolving stats to each target.

This patch adds breakpoints to each target's statistics so we can track how 
long it takes to resolve each breakpoint. It also includes the structured data 
for each breakpoint so the exact breakpoint details are logged to allow for 
reproduction of slow resolving breakpoints. Each target gets a new 
"breakpoints" array that contains breakpoint details. Each breakpoint has 
"details" which is the JSON representation of a serialized breakpoint resolver 
and filter, "id" which is the breakpoint ID, and "resolveTime" which is the 
time in seconds it took to resolve the breakpoint. A snippet of the new data is 
shown here:

  "targets": [
{
  "breakpoints": [
{
  "details": {...},
  "id": 1,
  "resolveTime": 0.000392915999
},
{
  "details": {...},
  "id": 2,
  "resolveTime": 0.000226791999
}
  ],
  "totalBreakpointResolveTime": 0.000619707996
}
  ]

This provides full details on exactly how breakpoints were set and how long it 
took to resolve them.

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/Breakpoint.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Target/Statistics.cpp
lldb/test/API/commands/statistics/basic/TestStats.py

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index f2e2a0d22784..40435d5c3d0f 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -22,6 +22,7 @@
 #include "lldb/Breakpoint/Stoppoint.h"
 #include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/SearchFilter.h"
+#include "lldb/Target/Statistics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/StructuredData.h"
@@ -576,6 +577,12 @@ class Breakpoint : public 
std::enable_shared_from_this,
   static lldb::BreakpointSP CopyFromBreakpoint(lldb::TargetSP new_target,
   const Breakpoint &bp_to_copy_from);
 
+  /// Get statistics associated with this breakpoint in JSON format.
+  llvm::json::Value GetStatistics();
+
+  /// Get the time it took to resolve all locations in this breakpoint.
+  StatsDuration GetResolveTime() const { return m_resolve_time; }
+
 protected:
   friend class Target;
   // Protected Methods
@@ -653,6 +660,8 @@ class Breakpoint : public 
std::enable_shared_from_this,
 
   BreakpointName::Permissions m_permissions;
 
+  StatsDuration m_resolve_time{0.0};
+
   void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind);
 
   void SendBreakpointChangedEvent(BreakpointEventData *data);

diff  --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index c644463719d4..d6acf659e852 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -439,12 +439,15 @@ BreakpointOptions &Breakpoint::GetOptions() { return 
m_options; }
 const BreakpointOptions &Breakpoint::GetOptions() const { return m_options; }
 
 void Breakpoint::ResolveBreakpoint() {
-  if (m_resolver_sp)
+  if (m_resolver_sp) {
+ElapsedTime elapsed(m_resolve_time);
 m_resolver_sp->ResolveBreakpoint(*m_filter_sp);
+  }
 }
 
 void Breakpoint::ResolveBreakpointInModules(
 ModuleList &module_list, BreakpointLocationCollection &new_locations) {
+  ElapsedTime elapsed(m_resolve_time);
   m_locations.StartRecordingNewLocations(new_locations);
 
   m_resolver_sp->ResolveBreakpointInModules(*m_filter_sp, module_list);
@@ -470,6 +473,7 @@ void Breakpoint::ResolveBreakpointInModules(ModuleList 
&module_list,
   } else
 delete new_locations_event;
 } else {
+  ElapsedTime elapsed(m_resolve_time);
   m_resolver_sp->ResolveBreakpointInModules(*m_filter_sp, module_list);
 }
   }
@@ -1086,3 +1090,34 @@ 
Breakpoint::BreakpointEventData::GetBreakpointLocationAtIndexFromEvent(
 
   return bp_loc_sp;
 }
+
+json::Value Breakpoint::GetStatistics() {
+  json::Object bp;
+  bp.try_emplace("id", GetID());
+  bp.try_emplace("resolveTime", m_resolve_time.count());
+  bp.try_emplace("numLocations", (int64_t)GetNumLocations());
+  bp.try_emplace("numResolvedLocations", (int64_t)GetNumResolvedLocations());
+  bp.try_emplace("internal", IsInternal());
+  if (!m_kind_description.empty())
+bp.try_emplace("kindDescription", m_kind_description);
+  // Put the full structured data for reproducing this breakpoint in a 
key/value
+  // pair named "details". This allows

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfb2549683260: Add breakpoint resolving stats to each target. 
(authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -283,3 +283,85 @@
 ]
 self.assertNotEqual(exe_module, None)
 self.verify_keys(exe_module, 'module dict for "%s"' % (exe), module_keys)
+
+def test_breakpoints(self):
+"""Test "statistics dump"
+
+Output expected to be something like:
+
+{
+  "modules" : [...],
+  "targets" : [
+{
+"firstStopTime": 0.3416449280001,
+"launchOrAttachTime": 0.3196960540001,
+"moduleIdentifiers": [...],
+"targetCreateTime": 0.00408630398
+"expressionEvaluation": {
+"failures": 0,
+"successes": 0
+},
+"frameVariable": {
+"failures": 0,
+"successes": 0
+},
+"breakpoints": [
+{
+"details": {...},
+"id": 1,
+"resolveTime": 2.65438675
+},
+{
+"details": {...},
+"id": 2,
+"resolveTime": 4.363258166997
+}
+]
+}
+],
+"totalDebugInfoByteSize": 182522234,
+"totalDebugInfoIndexTime": 2.33343,
+"totalDebugInfoParseTime": 8.212140024071,
+"totalSymbolTableParseTime": 0.123,
+"totalSymbolTableIndexTime": 0.234,
+"totalBreakpointResolveTime": 7.017644917001
+}
+
+"""
+target = self.createTestTarget()
+self.runCmd("b main.cpp:7")
+self.runCmd("b a_function")
+debug_stats = self.get_stats()
+debug_stat_keys = [
+'modules', 
+'targets',
+'totalSymbolTableParseTime',
+'totalSymbolTableIndexTime',
+'totalDebugInfoParseTime',
+'totalDebugInfoIndexTime',
+'totalDebugInfoByteSize',
+]
+self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+target_stats = debug_stats['targets'][0]
+keys_exist = [
+'breakpoints',
+'expressionEvaluation',
+'frameVariable',
+'targetCreateTime',
+'moduleIdentifiers',
+'totalBreakpointResolveTime',
+]
+self.verify_keys(target_stats, '"stats"', keys_exist, None)
+self.assertGreater(target_stats['totalBreakpointResolveTime'], 0.0)
+breakpoints = target_stats['breakpoints']
+bp_keys_exist = [
+'details',
+'id',
+'internal',
+'numLocations',
+'numResolvedLocations',
+'resolveTime'
+]
+for breakpoint in breakpoints:
+self.verify_keys(breakpoint, 'target_stats["breakpoints"]', 
+ bp_keys_exist, None)
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -80,6 +80,25 @@
   }
   target_metrics_json.try_emplace("targetCreateTime", m_create_time.count());
 
+  json::Array breakpoints_array;
+  double totalBreakpointResolveTime = 0.0;
+  // Rport both the normal breakpoint list and the internal breakpoint list.
+  for (int i = 0; i < 2; ++i) {
+BreakpointList &breakpoints = target.GetBreakpointList(i == 1);
+std::unique_lock lock;
+breakpoints.GetListMutex(lock);
+size_t num_breakpoints = breakpoints.GetSize();
+for (size_t i = 0; i < num_breakpoints; i++) {
+  Breakpoint *bp = breakpoints.GetBreakpointAtIndex(i).get();
+  breakpoints_array.push_back(bp->GetStatistics());
+  totalBreakpointResolveTime += bp->GetResolveTime().count();
+}
+  }
+
+  target_metrics_json.try_emplace("breakpoints", std::move(breakpoints_array));
+  target_metrics_json.try_emplace("totalBreakpointResolveTime",
+  totalBreakpo

[Lldb-commits] [PATCH] D112677: Remove unused ValueObjectDynamicValue::SetOwningSP & backing ivar

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, mib.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This owning_sp has no uses and the ValueObjectDynamicValue already tracks
its ownership through the parent it is passed when made.  I can't
find any vestiges of the use of this API, maybe it was from some
earlier design?

  

Resetting the backing ivar was the only job the destructor did, so I
set that to default as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112677

Files:
  lldb/include/lldb/Core/ValueObjectDynamicValue.h
  lldb/source/Core/ValueObjectDynamicValue.cpp


Index: lldb/source/Core/ValueObjectDynamicValue.cpp
===
--- lldb/source/Core/ValueObjectDynamicValue.cpp
+++ lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -36,10 +36,6 @@
   SetName(parent.GetName());
 }
 
-ValueObjectDynamicValue::~ValueObjectDynamicValue() {
-  m_owning_valobj_sp.reset();
-}
-
 CompilerType ValueObjectDynamicValue::GetCompilerTypeImpl() {
   const bool success = UpdateValueIfNeeded(false);
   if (success) {
Index: lldb/include/lldb/Core/ValueObjectDynamicValue.h
===
--- lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -32,7 +32,7 @@
 /// set lldb type.
 class ValueObjectDynamicValue : public ValueObject {
 public:
-  ~ValueObjectDynamicValue() override;
+  ~ValueObjectDynamicValue() = default;
 
   llvm::Optional GetByteSize() override;
 
@@ -68,14 +68,6 @@
 
   lldb::ValueObjectSP GetStaticValue() override { return m_parent->GetSP(); }
 
-  void SetOwningSP(lldb::ValueObjectSP &owning_sp) {
-if (m_owning_valobj_sp == owning_sp)
-  return;
-
-assert(m_owning_valobj_sp.get() == nullptr);
-m_owning_valobj_sp = owning_sp;
-  }
-
   bool SetValueFromCString(const char *value_str, Status &error) override;
 
   bool SetData(DataExtractor &data, Status &error) override;
@@ -117,7 +109,6 @@
 
   Address m_address; ///< The variable that this value object is based upon
   TypeAndOrName m_dynamic_type_info; // We can have a type_sp or just a name
-  lldb::ValueObjectSP m_owning_valobj_sp;
   lldb::DynamicValueType m_use_dynamic;
   TypeImpl m_type_impl;
 


Index: lldb/source/Core/ValueObjectDynamicValue.cpp
===
--- lldb/source/Core/ValueObjectDynamicValue.cpp
+++ lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -36,10 +36,6 @@
   SetName(parent.GetName());
 }
 
-ValueObjectDynamicValue::~ValueObjectDynamicValue() {
-  m_owning_valobj_sp.reset();
-}
-
 CompilerType ValueObjectDynamicValue::GetCompilerTypeImpl() {
   const bool success = UpdateValueIfNeeded(false);
   if (success) {
Index: lldb/include/lldb/Core/ValueObjectDynamicValue.h
===
--- lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -32,7 +32,7 @@
 /// set lldb type.
 class ValueObjectDynamicValue : public ValueObject {
 public:
-  ~ValueObjectDynamicValue() override;
+  ~ValueObjectDynamicValue() = default;
 
   llvm::Optional GetByteSize() override;
 
@@ -68,14 +68,6 @@
 
   lldb::ValueObjectSP GetStaticValue() override { return m_parent->GetSP(); }
 
-  void SetOwningSP(lldb::ValueObjectSP &owning_sp) {
-if (m_owning_valobj_sp == owning_sp)
-  return;
-
-assert(m_owning_valobj_sp.get() == nullptr);
-m_owning_valobj_sp = owning_sp;
-  }
-
   bool SetValueFromCString(const char *value_str, Status &error) override;
 
   bool SetData(DataExtractor &data, Status &error) override;
@@ -117,7 +109,6 @@
 
   Address m_address; ///< The variable that this value object is based upon
   TypeAndOrName m_dynamic_type_info; // We can have a type_sp or just a name
-  lldb::ValueObjectSP m_owning_valobj_sp;
   lldb::DynamicValueType m_use_dynamic;
   TypeImpl m_type_impl;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112677: Remove unused ValueObjectDynamicValue::SetOwningSP & backing ivar

[Greg Clayton via Phabricator via lldb-commits]

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

I am sure you have checked the Swift codebase as well for any uses. If so LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112677

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

[Lldb-commits] [PATCH] D105470: [lldb] Clear children of ValueObject on value update

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D105470#2868858 , @werat wrote:

> Thanks for the explanation! But at this point I feel I'm a bit confused about 
> how it all _supposed_ to work in the current design :)

Apparently I lost track of this review...

> If I understand correctly, there are four "types" of values from the user 
> (API) perspective:
>
> 1. `ExpressionResult` -- value returned by `SBFrame::EvaluateExpression()`

This is the one that tis expected to be "constant".  After all, expression 
results aren't necessarily anything in the target, they could be an int 
returned by a function call that is only in the expression result.
Moreover, this ValueObject represents the "results of an expression".  It 
really doesn't make sense to change that.  And even if the expression result is 
just some local variable, we want users to be able to refer back to the results 
of expressions even when they have left the frame where the expression was 
evaluated.  So these ones need to be "frozen" values.

Note there is a bunch of code in the ValueObjectConstResult that tracks the 
"live address" if the expression result was something in memory.  That's so 
that you can do things like *$1 and have then do the right thing.

And, there's another complexity which is that the ValueObjectConstResult class 
got reused for a bunch of things that don't have this restriction, so the 
internal policy is pretty confusing.  I'm trying to find some time to clean 
this up a bit in the near term.

> 2. `ExpressionPersistentVariable` -- value created by the expression via 
> `auto $name = ...` syntax. Can be obtained by `SBFrame::FindValue("$name", 
> lldb::eValueTypeConstResult)`.

These ones are supposed to act as though the user had declared an exported 
global variable of this name and type.  So it should be modifiable.  As I said 
above, it's really confusing that this is a ConstResult, which it clearly 
isn't...

> 3. "Const value" -- value created by `SBTarget::CreateValueFromData()` or 
> `SBTarget::CreateValueFromAddress`

Creating a value from Data is making a ValueObject that only makes sense to 
lldb.  This is data in lldb's memory.  But these entities are really for use by 
other ValueObject entities, and they would know what the right policy for 
recalculation should be.

CreateValueFromAddress is like a the Expression Persistent Variables.  It 
refers to an address in the target, and isn't associated with a frame, so it 
should just reflect what's in that memory location, and writing makes sense as 
well.

> 4. "Variable reference" -- value returned by `SBFrame::FindVariable()`

These ones are pretty self evident.  We do allow you to modify the state of 
locals, so that should be hooked up.  If the underlying variable is a stack 
local, you it doesn't make sense to update or change them once their frame has 
been pushed off the stack.  But if the variable is a global, then it should be 
valid till the library it lives in gets unloaded.  These all have update points 
saying what the criteria for their validity are.

> For which of these value the following test is supposed to work?
>
>   struct Foo { int x; };
>   Foo* f1 = { .x = 1}
>   Foo* f2 = { .x = 2}  # pseudo-C for simplicity
>   
>   f1_ref = ...  # Get a value that holds the value of `f1` using one of the 
> four methods described above
>   print(f1_ref.GetChild(0))  # '1'
>   f1_ref.SetValueFromCString(frame.FindVariable('f2').value)
>   print(f1_ref.GetChild(0))  # '2'
>
> My experiments show that it works for "variable references" and "const 
> values" created by 
> `CreateValueFromAddress` (but _not_ `CreateValueFromData`).
> UPD: it seems values created `CreateValueFromAddress` actually behave like 
> "variable references". Modifying their value will modify the underlying data 
> directly.
>
> If I understand your comment correctly, you're saying it should work only for 
> `ExpressionPersistentVariable` values (#2). Is that right?

Sorry if I wasn't clear.  From the comments above, you should be able to update 
everything but expression result variables.   Updating the From Data ones is a 
little less clear, but they are mostly for internal uses - like for handing out 
Synthetic Children.

> I don't have the full picture about the internal implementation and all the 
> use cases, but as a user I would expect it to work for at least #2, #3 and 
> #4. Afaik there's no API to fully distinguish between these kinds of values, 
> so I find it confusing why `SBValue::SetData()` would be allowed for some 
> values and not allowed for others. If I can create a value using 
> `CreateValueFromData` and then there's a method `SetValueFromCString`, then I 
> don't see why it should not be allowed (apart from implementation 
> complexity/consistency reasons).
>
> What do you think? How should we proceed with this?

Note, however, that there is one more detail of importance in this system.  For 
variables that represent valid entiti

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Note, the breakpoint serialization for breakpoints only serializes the 
filter/resolver and the breakpoint specific options.  It doesn't do anything 
with locations or their options.  After all, you when you go to reset a 
breakpoint in another session (which is what this serialization is for) you 
have no way of knowing what locations you are going to find.

So if you want to record the locations (for instance to make sure that your 
recreation actually did the same work) then you will have to record the 
locations some other way.  However, you do record the number of locations, 
which is probably a good enough signal that you aren't recreating what you 
thought you were.  So maybe you don't need to worry about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

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

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D112587#3092120 , @jingham wrote:

> Note, the breakpoint serialization for breakpoints only serializes the 
> filter/resolver and the breakpoint specific options.  It doesn't do anything 
> with locations or their options.  After all, you when you go to reset a 
> breakpoint in another session (which is what this serialization is for) you 
> have no way of knowing what locations you are going to find.

Good to know!

> So if you want to record the locations (for instance to make sure that your 
> recreation actually did the same work) then you will have to record the 
> locations some other way.  However, you do record the number of locations, 
> which is probably a good enough signal that you aren't recreating what you 
> thought you were.  So maybe you don't need to worry about this.

I am mainly concerned with the overall details of a breakpoint, and usually for 
the breakpoints that have no locations due to a source mapping error, or lack 
of debug information. Both stats are available (debug info size + breakpoint 
filter/resolver details) so that will help us figure out why things are not 
working for people. I had locations before, but I am not sure we need that 
information just yet so I will leave that for later if we do end up needing it 
for some reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

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

[Lldb-commits] [PATCH] D112587: Add breakpoint resolving stats to each target.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D112587#3092125 , @clayborg wrote:

> In D112587#3092120 , @jingham wrote:
>
>> Note, the breakpoint serialization for breakpoints only serializes the 
>> filter/resolver and the breakpoint specific options.  It doesn't do anything 
>> with locations or their options.  After all, you when you go to reset a 
>> breakpoint in another session (which is what this serialization is for) you 
>> have no way of knowing what locations you are going to find.
>
> Good to know!
>
>> So if you want to record the locations (for instance to make sure that your 
>> recreation actually did the same work) then you will have to record the 
>> locations some other way.  However, you do record the number of locations, 
>> which is probably a good enough signal that you aren't recreating what you 
>> thought you were.  So maybe you don't need to worry about this.
>
> I am mainly concerned with the overall details of a breakpoint, and usually 
> for the breakpoints that have no locations due to a source mapping error, or 
> lack of debug information. Both stats are available (debug info size + 
> breakpoint filter/resolver details) so that will help us figure out why 
> things are not working for people. I had locations before, but I am not sure 
> we need that information just yet so I will leave that for later if we do end 
> up needing it for some reason.

This seems like a reasonable choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112587

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

[Lldb-commits] [PATCH] D112683: Add unix signal hit counts to the target statistics.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: JDevlieghere, jingham, wallace, aadsm.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Android and other platforms make wide use of signals when running applications 
and this can slow down debug sessions. Tracking this statistic can help us to 
determine why a debug session is slow.

The new data appears inside each target object and reports the signal hit 
counts:

  "signals": [
{
  "SIGSTOP": 1
},
{
  "SIGUSR1": 1
}
  ],


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112683

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Target/Statistics.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/test/API/functionalities/signal/TestSendSignal.py

Index: lldb/test/API/functionalities/signal/TestSendSignal.py
===
--- lldb/test/API/functionalities/signal/TestSendSignal.py
+++ lldb/test/API/functionalities/signal/TestSendSignal.py
@@ -95,6 +95,10 @@
 thread.GetStopReasonDataAtIndex(0), lldbutil.get_signal_number('SIGUSR1'),
 "The stop signal was SIGUSR1")
 
+self.match("statistics dump",
+   [r'"signals": \[', r'"SIGUSR1": 1'])
+
+
 def match_state(self, process_listener, expected_state):
 num_seconds = 5
 broadcaster = self.process().GetBroadcaster()
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
+using namespace llvm;
 
 UnixSignals::Signal::Signal(const char *name, bool default_suppress,
 bool default_stop, bool default_notify,
@@ -312,3 +313,20 @@
 
   return result;
 }
+
+void UnixSignals::IncrementSignalHitCount(int signo) {
+  collection::iterator pos = m_signals.find(signo);
+  if (pos != m_signals.end())
+pos->second.m_hit_count += 1;
+}
+
+json::Value UnixSignals::GetHitCountStatistics() const {
+  json::Array json_signals;
+  for (const auto &pair: m_signals) {
+if (pair.second.m_hit_count > 0)
+  json_signals.emplace_back(json::Object{
+{ pair.second.m_name.GetCString(), pair.second.m_hit_count }
+  });
+  }
+  return std::move(json_signals);
+}
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -307,7 +307,7 @@
 
   // There's one other complication here.  We may have run an async
   // breakpoint callback that said we should stop.  We only want to
-  // override that if another breakpoint action says we shouldn't 
+  // override that if another breakpoint action says we shouldn't
   // stop.  If nobody else has an opinion, then we should stop if the
   // async callback says we should.  An example of this is the async
   // shared library load notification breakpoint and the setting
@@ -425,7 +425,7 @@
 }
 
 internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal();
-
+
 // First run the precondition, but since the precondition is per
 // breakpoint, only run it once per breakpoint.
 std::pair::iterator, bool> result =
@@ -535,7 +535,7 @@
   else
 actually_said_continue = true;
 }
-  
+
 // If we are going to stop for this breakpoint, then remove the
 // breakpoint.
 if (callback_says_stop && bp_loc_sp &&
@@ -579,7 +579,7 @@
 // Override should_stop decision when we have completed step plan
 // additionally to the breakpoint
 m_should_stop = true;
-
+
 // We know we're stopping for a completed plan and we don't want to
 // show the breakpoint stop, so compute the public stop info immediately
 // here.
@@ -615,7 +615,7 @@
   // performing watchpoint actions.
   class WatchpointSentry {
   public:
-WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp), 
+WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp),
  watchpoint_sp(w_sp) {
   if (process_sp && watchpoint_sp) {
 const bool notify = false;
@@ -624,7 +624,7 @@
 process_sp->AddPreResumeAction(SentryPreResumeAction, this);
   }
 }
-
+
 void DoReenable() {
   if (process_sp && watchpoint_sp) {
 bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode();
@@ -637,13 +637,13 @@
 }
   }
 }
-
+
 ~WatchpointSentry() {
 DoReenable();
 if (process_sp)
 process_sp->ClearPr

[Lldb-commits] [PATCH] D112683: Add unix signal hit counts to the target statistics.

[walter erquinigo via Phabricator via lldb-commits]

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

great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112683

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

[Lldb-commits] [PATCH] D112683: Add unix signal hit counts to the target statistics.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG130055647922: Add unix signal hit counts to the target 
statistics. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112683

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Target/Statistics.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/test/API/functionalities/signal/TestSendSignal.py

Index: lldb/test/API/functionalities/signal/TestSendSignal.py
===
--- lldb/test/API/functionalities/signal/TestSendSignal.py
+++ lldb/test/API/functionalities/signal/TestSendSignal.py
@@ -95,6 +95,10 @@
 thread.GetStopReasonDataAtIndex(0), lldbutil.get_signal_number('SIGUSR1'),
 "The stop signal was SIGUSR1")
 
+self.match("statistics dump",
+   [r'"signals": \[', r'"SIGUSR1": 1'])
+
+
 def match_state(self, process_listener, expected_state):
 num_seconds = 5
 broadcaster = self.process().GetBroadcaster()
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
+using namespace llvm;
 
 UnixSignals::Signal::Signal(const char *name, bool default_suppress,
 bool default_stop, bool default_notify,
@@ -312,3 +313,20 @@
 
   return result;
 }
+
+void UnixSignals::IncrementSignalHitCount(int signo) {
+  collection::iterator pos = m_signals.find(signo);
+  if (pos != m_signals.end())
+pos->second.m_hit_count += 1;
+}
+
+json::Value UnixSignals::GetHitCountStatistics() const {
+  json::Array json_signals;
+  for (const auto &pair: m_signals) {
+if (pair.second.m_hit_count > 0)
+  json_signals.emplace_back(json::Object{
+{ pair.second.m_name.GetCString(), pair.second.m_hit_count }
+  });
+  }
+  return std::move(json_signals);
+}
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -307,7 +307,7 @@
 
   // There's one other complication here.  We may have run an async
   // breakpoint callback that said we should stop.  We only want to
-  // override that if another breakpoint action says we shouldn't 
+  // override that if another breakpoint action says we shouldn't
   // stop.  If nobody else has an opinion, then we should stop if the
   // async callback says we should.  An example of this is the async
   // shared library load notification breakpoint and the setting
@@ -425,7 +425,7 @@
 }
 
 internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal();
-
+
 // First run the precondition, but since the precondition is per
 // breakpoint, only run it once per breakpoint.
 std::pair::iterator, bool> result =
@@ -535,7 +535,7 @@
   else
 actually_said_continue = true;
 }
-  
+
 // If we are going to stop for this breakpoint, then remove the
 // breakpoint.
 if (callback_says_stop && bp_loc_sp &&
@@ -579,7 +579,7 @@
 // Override should_stop decision when we have completed step plan
 // additionally to the breakpoint
 m_should_stop = true;
-
+
 // We know we're stopping for a completed plan and we don't want to
 // show the breakpoint stop, so compute the public stop info immediately
 // here.
@@ -615,7 +615,7 @@
   // performing watchpoint actions.
   class WatchpointSentry {
   public:
-WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp), 
+WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp),
  watchpoint_sp(w_sp) {
   if (process_sp && watchpoint_sp) {
 const bool notify = false;
@@ -624,7 +624,7 @@
 process_sp->AddPreResumeAction(SentryPreResumeAction, this);
   }
 }
-
+
 void DoReenable() {
   if (process_sp && watchpoint_sp) {
 bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode();
@@ -637,13 +637,13 @@
 }
   }
 }
-
+
 ~WatchpointSentry() {
 DoReenable();
 if (process_sp)
 process_sp->ClearPreResumeAction(SentryPreResumeAction, this);
 }
-
+
 static bool SentryPreResumeAction(void *sentry_void) {
 WatchpointSentry *sentry = (WatchpointSentry *) sentry_void;
 sentry->DoReenable();
@@ -724,14 +724,14 @@
 // course of this code.  Also by default we're going to stop, so set that
 //

[Lldb-commits] [lldb] 1300556 - Add unix signal hit counts to the target statistics.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2021-10-27T22:31:14-07:00
New Revision: 130055647922ffa9dd9abd5a838727773f73c48a

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

LOG: Add unix signal hit counts to the target statistics.

Android and other platforms make wide use of signals when running applications 
and this can slow down debug sessions. Tracking this statistic can help us to 
determine why a debug session is slow.

The new data appears inside each target object and reports the signal hit 
counts:

  "signals": [
{
  "SIGSTOP": 1
},
{
  "SIGUSR1": 1
}
  ],

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

Added: 


Modified: 
lldb/include/lldb/Target/UnixSignals.h
lldb/source/Target/Statistics.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/UnixSignals.cpp
lldb/test/API/functionalities/signal/TestSendSignal.py

Removed: 




diff  --git a/lldb/include/lldb/Target/UnixSignals.h 
b/lldb/include/lldb/Target/UnixSignals.h
index 6fecdda12defc..1c91c9fdd4895 100644
--- a/lldb/include/lldb/Target/UnixSignals.h
+++ b/lldb/include/lldb/Target/UnixSignals.h
@@ -16,6 +16,7 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/JSON.h"
 
 namespace lldb_private {
 
@@ -80,6 +81,18 @@ class UnixSignals {
 
   void RemoveSignal(int signo);
 
+  /// Track how many times signals are hit as stop reasons.
+  void IncrementSignalHitCount(int signo);
+
+  /// Get the hit count statistics for signals.
+  ///
+  /// Gettings statistics on the hit counts of signals can help explain why 
some
+  /// debug sessions are slow since each stop takes a few hundred ms and some
+  /// software use signals a lot and can cause slow debugging performance if
+  /// they are used too often. Even if a signal is not stopped at, it will auto
+  /// continue the process and a delay will happen.
+  llvm::json::Value GetHitCountStatistics() const;
+
   // Returns a current version of the data stored in this class. Version gets
   // incremented each time Set... method is called.
   uint64_t GetVersion() const;
@@ -99,6 +112,7 @@ class UnixSignals {
 ConstString m_name;
 ConstString m_alias;
 std::string m_description;
+uint32_t m_hit_count = 0;
 bool m_suppress : 1, m_stop : 1, m_notify : 1;
 
 Signal(const char *name, bool default_suppress, bool default_stop,

diff  --git a/lldb/source/Target/Statistics.cpp 
b/lldb/source/Target/Statistics.cpp
index a7207ac5d990d..41639264a949b 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -11,7 +11,9 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/SymbolFile.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/UnixSignals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -95,6 +97,13 @@ json::Value TargetStats::ToJSON(Target &target) {
 }
   }
 
+  ProcessSP process_sp = target.GetProcessSP();
+  if (process_sp) {
+UnixSignalsSP unix_signals_sp = process_sp->GetUnixSignals();
+if (unix_signals_sp)
+  target_metrics_json.try_emplace("signals",
+  
unix_signals_sp->GetHitCountStatistics());
+  }
   target_metrics_json.try_emplace("breakpoints", std::move(breakpoints_array));
   target_metrics_json.try_emplace("totalBreakpointResolveTime",
   totalBreakpointResolveTime);

diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 8258705990eb0..ef7dff4052ead 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -307,7 +307,7 @@ class StopInfoBreakpoint : public StopInfo {
 
   // There's one other complication here.  We may have run an async
   // breakpoint callback that said we should stop.  We only want to
-  // override that if another breakpoint action says we shouldn't 
+  // override that if another breakpoint action says we shouldn't
   // stop.  If nobody else has an opinion, then we should stop if the
   // async callback says we should.  An example of this is the async
   // shared library load notification breakpoint and the setting
@@ -425,7 +425,7 @@ class StopInfoBreakpoint : public StopInfo {
 }
 
 internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal();
-
+
 // First run the precondition, but since the precondition is per
 // breakpoint, only run it once per breakpoint.
 std::pair::iterator, bool> result =
@@ -535,7 +535,7 @@ class StopInfoBreakpoint : public StopInfo {
   

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: wallace, aadsm, jingham, JDevlieghere.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Many target settings can be set that can affect how breakpoints are found 
during a debug session. Having these settings along with the statistics are 
important for being able to reproduce issues clients are having.

A new "settings" key/value pair is added to each target dictionary that 
contains settings:

  "settings": {
"target.arg0": "/tmp/a.out",
"target.clang-module-search-paths": [
  "/tmp",
  "/var"
],
"target.debug-file-search-paths": [
  "/tmp",
  "/var"
],
"target.exec-search-paths": [
  "/tmp",
  "/var"
],
"target.inline-breakpoint-strategy": "headers",
"target.preload-symbols": true,
"target.run-args": [
  "a",
  "b",
  "c"
],
"target.skip-prologue": true,
"target.source-map": [
  [
"/tmp",
"/var"
  ]
]
  },


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112691

Files:
  lldb/include/lldb/Core/FileSpecList.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,5 +1,6 @@
 import lldb
 import json
+import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -154,7 +155,7 @@
 target = self.createTestTarget()
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -217,7 +218,7 @@
   lldb.SBFileSpec("main.c"))
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -255,7 +256,7 @@
 target = self.createTestTarget(file_path=exe)
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -315,7 +316,7 @@
 "details": {...},
 "id": 2,
 "resolveTime": 4.363258166997
-}
+}
 ]
 }
 ],
@@ -333,7 +334,7 @@
 self.runCmd("b a_function")
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -363,5 +364,134 @@
 'resolveTime'
 ]
 for breakpoint in breakpoints:
-self.verify_keys(breakpoint, 'target_stats["breakpoints"]', 
+self.verify_keys(breakpoint, 'target_stats["breakpoints"]',
  bp_keys_exist, None)
+
+def test_settings(self):
+"""Test "statistics dump --settings"
+
+Output expected to be something like:
+
+(lldb) statistics dump --settings
+{
+  "targetCreateTime": 0.265668995,
+  "totalBreakpointResolveTime": 0.00314094199,
+  "totalDebugInfoIndexTime": 0,
+  "totalDebugInfoParseTime": 0.000372883001,
+  "totalDebugInfoSize": 2193,
+  "totalSymbolTableIndexTime": 0.0568344889,
+  "totalSymbolTableParseTime": 0.09397942199979
+  "settings": {
+"target.arg0": "/.../commands/target/metrics/TestStats.test_settings/a.out",
+"target.clang-module-search-paths": [],
+"target.debug-file-search-paths": [],
+"target.exec-search-paths": [
+  "/.../commands/target/metrics/TestStats.test_settings"
+],
+"target.inline-breakpoint-strategy": "always",
+"target.preload-symbols": true,
+"target.run-args": [],
+"target.skip-prologue": true,
+"target.source-map": []
+},
+  }
+
+"""
+exe = self.getBuildArtifact("a.out")
+exe_dir = os.path.dirname(exe)
+path1 = self.getSourceDir()
+path2 = exe_dir
+target = self.createTestTarget(file_path=exe)
+debug_stat

[Lldb-commits] [PATCH] D112629: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Host/common/Socket.cpp:198-199
   } else {
 // If this was unsuccessful, then check if it's simply a signed 32-bit
 // integer, representing a port with an empty host.
+if (to_integer(host_and_port, ret.port, 10))

I guess this isn't true anymore.


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

https://reviews.llvm.org/D112629

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

[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for doing this. Just a couple of small remarks.




Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:35
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+namespace {
+

We don't use [[ https://llvm.org/docs/CodingStandards.html#anonymous-namespaces 
| anonymous namespaces]] like this. `static` is sufficient.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:76-85
+if (ElemType == StringPrinter::StringElementType::UTF8) {
+  options.SetPrefixToken("u8");
+  valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF16) {
+  options.SetPrefixToken("u");
+  valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF32) {

Maybe a helper function like `pair 
getElementTraits(StringElementType)` would reduce the repetition further? Or 
possibly it could be a static array indexed by `ElemType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112658

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