[Lldb-commits] [lldb] 5e111eb - Migrate away from the soft-deprecated functions in APInt.h (NFC)

[Kazu Hirata via lldb-commits]

Author: Kazu Hirata
Date: 2023-02-20T00:58:29-08:00
New Revision: 5e111eb275eee3bec1123b4b85606328017e5ee5

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

LOG: Migrate away from the soft-deprecated functions in APInt.h (NFC)

Note that those functions on the left hand side are soft-deprecated in
favor of those on the right hand side:

  getMinSignedBits -> getSignificantBits
  getNullValue -> getZero
  isNullValue  -> isZero
  isOneValue   -> isOne

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Utility/Scalar.cpp
llvm/unittests/ADT/APIntTest.cpp
llvm/unittests/IR/ConstantRangeTest.cpp
polly/unittests/Isl/IslTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 717456698eb23..362ce2b200ae1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2707,7 +2707,7 @@ llvm::Expected 
DWARFASTParserClang::ExtractIntFromFormValue(
   // For signed types, ask APInt how many bits are required to represent the
   // signed integer.
   const unsigned required_bits =
-  is_unsigned ? result.getActiveBits() : result.getMinSignedBits();
+  is_unsigned ? result.getActiveBits() : result.getSignificantBits();
 
   // If the input value doesn't fit into the integer type, return an error.
   if (required_bits > type_bits) {

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 19e00e111be52..4b80f6adca06a 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -145,7 +145,7 @@ bool Scalar::IsZero() const {
   case e_void:
 break;
   case e_int:
-return m_integer.isNullValue();
+return m_integer.isZero();
   case e_float:
 return m_float.isZero();
   }

diff  --git a/llvm/unittests/ADT/APIntTest.cpp 
b/llvm/unittests/ADT/APIntTest.cpp
index ad0a280ae2a26..d8d7bfa27a3f3 100644
--- a/llvm/unittests/ADT/APIntTest.cpp
+++ b/llvm/unittests/ADT/APIntTest.cpp
@@ -166,7 +166,7 @@ TEST(APIntTest, i128_PositiveCount) {
   EXPECT_EQ(96u, s128.countl_zero());
   EXPECT_EQ(0u, s128.countl_one());
   EXPECT_EQ(32u, s128.getActiveBits());
-  EXPECT_EQ(33u, s128.getMinSignedBits());
+  EXPECT_EQ(33u, s128.getSignificantBits());
   EXPECT_EQ(1u, s128.countr_zero());
   EXPECT_EQ(0u, s128.countr_one());
   EXPECT_EQ(30u, s128.popcount());
@@ -176,7 +176,7 @@ TEST(APIntTest, i128_PositiveCount) {
   EXPECT_EQ(0u, s128.countl_zero());
   EXPECT_EQ(66u, s128.countl_one());
   EXPECT_EQ(128u, s128.getActiveBits());
-  EXPECT_EQ(63u, s128.getMinSignedBits());
+  EXPECT_EQ(63u, s128.getSignificantBits());
   EXPECT_EQ(1u, s128.countr_zero());
   EXPECT_EQ(0u, s128.countr_one());
   EXPECT_EQ(96u, s128.popcount());
@@ -200,7 +200,7 @@ TEST(APIntTest, i256) {
   EXPECT_EQ(190u, s256.countl_zero());
   EXPECT_EQ(0u, s256.countl_one());
   EXPECT_EQ(66u, s256.getActiveBits());
-  EXPECT_EQ(67u, s256.getMinSignedBits());
+  EXPECT_EQ(67u, s256.getSignificantBits());
   EXPECT_EQ(0u, s256.countr_zero());
   EXPECT_EQ(4u, s256.countr_one());
   EXPECT_EQ(8u, s256.popcount());
@@ -209,7 +209,7 @@ TEST(APIntTest, i256) {
   EXPECT_EQ(0u, s256.countl_zero());
   EXPECT_EQ(196u, s256.countl_one());
   EXPECT_EQ(256u, s256.getActiveBits());
-  EXPECT_EQ(61u, s256.getMinSignedBits());
+  EXPECT_EQ(61u, s256.getSignificantBits());
   EXPECT_EQ(0u, s256.countr_zero());
   EXPECT_EQ(4u, s256.countr_one());
   EXPECT_EQ(200u, s256.popcount());
@@ -2759,7 +2759,7 @@ TEST(APIntTest, RoundingSDiv) {
   APInt QuoTowardZero = A.sdiv(B);
   {
 APInt Quo = APIntOps::RoundingSDiv(A, B, APInt::Rounding::UP);
-if (A.srem(B).isNullValue()) {
+if (A.srem(B).isZero()) {
   EXPECT_EQ(QuoTowardZero, Quo);
 } else if (A.isNegative() !=
B.isNegative()) { // if the math quotient is negative.
@@ -2770,7 +2770,7 @@ TEST(APIntTest, RoundingSDiv) {
   }
   {
 APInt Quo = APIntOps::RoundingSDiv(A, B, APInt::Rounding::DOWN);
-if (A.srem(B).isNullValue()) {
+if (A.srem(B).isZero()) {
   EXPECT_EQ(QuoTowardZero, Quo);
 } else if (A.isNegative() !=
B.isNegative()) { // if the math quotient is negative.
@@ -2929,12 +2929,12 @@ TEST(APIntTest, MultiplicativeInverseExaustive) {
   .multiplicativeInverse(APInt::getSignedMinValue(BitWidth + 1))
   .trunc(BitWidth);
   APInt One = V * MulInv;
-  if (!V.isNullValue() && V.countr_zero() == 0) {
+  if (!V.isZero() && V.countr_zero() == 0) {
 // Multiplicative inverse exists for all odd numb

[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

[Callum Macmillan via Phabricator via lldb-commits]

cimacmillan added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140630

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Before this patch, lldb did not send signal filtering information (QPassSignals 
packet) to lldb-server on remote platforms until signal settings were 
explicitly changed. Patch changes last sent signals version to be initialized 
with an invalid value instead of 0, so that the signal filtering information is 
sent to the server when the version is actually 0, which happens on remote 
platforms when the signal structure is copied and the version is reset. Also 
changes Process so that the information is sent not only right after launch, 
but right after attach too to be more consistent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144390

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
@@ -0,0 +1,31 @@
+"""Test that GDBRemoteProcess sends default signal filtering info when 
necessary"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestGDBRemoteSignalFiltering(GDBRemoteTestBase):
+
+def test_signals_sent_on_connect(self):
+"""Test that signal filtering info is sent on connect"""
+class Responder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;QPassSignals+"
+
+def respond(self, packet):
+if packet == "QPassSignals":
+return self.QPassSignals()
+return MockGDBServerResponder.respond(self, packet)
+
+def QPassSignals(self):
+return "OK"
+
+self.server.responder = Responder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertGreater(
+len([p for p in self.server.responder.packetLog if 
p.startswith("QPassSignals:")]),
+0)
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2592,7 +2592,7 @@
 if (!m_os_up)
   LoadOperatingSystemPlugin(false);
 
-// We successfully launched the process and stopped, now it the
+// We successfully launched the process and stopped, now it is the
 // right time to set up signal filters before resuming.
 UpdateAutomaticSignalFiltering();
 return Status();
@@ -3026,6 +3026,10 @@
 : "");
 }
   }
+
+  // We successfully attached to the process and stopped, now it is the
+  // right time to set up signal filters before resuming.
+  UpdateAutomaticSignalFiltering();
 }
 
 Status Process::ConnectRemote(llvm::StringRef remote_url) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -420,7 +420,7 @@
   // For ProcessGDBRemote only
   std::string m_partial_profile_data;
   std::map m_thread_id_to_used_usec_map;
-  uint64_t m_last_signals_version = 0;
+  uint64_t m_last_signals_version = UINT64_MAX;
 
   static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
@@ -0,0 +1,31 @@
+"""Test that GDBRemoteProcess sends default signal filtering info when necessary"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestGDBRemoteSignalFiltering(GDBRemoteTestBase):
+
+def test_signals_sent_on_connect(self):
+"""Test that signal filtering info is sent on connect"""
+class Responder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;QPassSignals+"
+
+def respond(self, packet):
+

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: DavidSpickett, clayborg, k8stone, davide.
kpdev42 added a project: LLDB.
Herald added subscribers: JDevlieghere, s.egerton, dmgreen, simoncook, emaste.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added subscribers: lldb-commits, pcwang-thead.

Before this patch, stepping off a breakpoint in lldb might end up inside a 
signal handler if a signal was received, which would result in continuing and 
hitting the same breakpoint again, while on the surface no instruction was 
executed and the user is not interested in that specific signal or its 
handling. This patch uses the machinery set up for software single-stepping to 
circumvent this behavior. This changes what a eStateStepping in lldb-server 
does to a thread on linux platforms: if a single-step is requested right after 
an ignored (pass=true, stop=false, notify=false) signal is received by the 
inferior, then a breakpoint is installed on the current pc, and the thread is 
continued (not single-stepped). When that breakpoint hits (presumably after the 
user signal handler returns or immediately after the continue, if the handler 
is not installed), it is removed, the stop is ignored and the thread is 
continued with single-stepping again. If a stop occurs inside the handler, then 
the stepping ends there, and the breakpoint is removed as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144392

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeProcessContinueUntilBreakpoint.cpp
  lldb/source/Plugins/Process/Utility/NativeProcessContinueUntilBreakpoint.h
  lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
  lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h
  lldb/test/API/functionalities/thread/signal_during_breakpoint_step/Makefile
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/SignalDuringBreakpointStepTestCase.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepIn.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepOut.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepOver.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepUntil.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepOut.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepOver.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepUntil.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalStepOverHandler.py
  lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp

Index: lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp
@@ -0,0 +1,166 @@
+// This test is intended to create a situation in which signals are received by
+// a thread while it is stepping off a breakpoint. The intended behavior is to
+// skip the handler and to not stop at the breakpoint we are trying to step off
+// the second time, as the corresponding instruction was not executed anyway.
+// If a breakpoint is hit inside the handler, the breakpoint on the line with
+// the original instruction should be hit when the handler is finished.
+//
+// This checks stepping off breakpoints set on single instructions and function
+// calls as well, to see a potential pc change when single-stepping.
+
+#include "pseudo_barrier.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+pseudo_barrier_t g_barrier;
+std::atomic g_send_signals;
+int g_action = 0;
+int g_signal_count = 0;
+int g_cur_iter = 0;
+
+// number of times to repeat the action
+int NUM_ITERS = 0;
+// number of times to send a signal per action
+int NUM_SIGNAL_ITERS = 0;
+// number of microseconds to wait between new action checks
+int SIGNAL_ITER_BACKOFF_US = 0;
+// number of microseconds to wait before sending another signal
+int SIGNAL_SIGNAL_ITER_BACKOFF_US = 0;
+// number of microseconds to wait inside the signal handler
+int HANDLER_SLEEP_US = 0;
+
+using action_t = void (*)();
+
+void do_action_func(action_t action) {

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

1. There is a situation where the patch might go wrong. When stepping while 
inside:
  - a signal handler when `SA_NODEFER` flag is set (the same signal can be 
received while it is still being handled), and the signal is received again;
  - a handler that is set on multiple signals and they are not masked, and 
these signals are received one after another;
  - a handler that happens to call the current function

recursion happens and stopping at this specifically set up breakpoint on pc is 
not valid. With this patch this would result in unexpected stops, though this 
situation is very specific. Would it make sense to complicate this logic and 
store current `sp` or `fp` values from `NativeRegisterContext` and compare if 
they are the same?

2. The basic behavior is pretty easy to test by sending a lot of signals, but 
testing specific cases like setting the temporary breakpoint on an address 
where there is already another breakpoint is tough. One test was added for 
this, but it does not actually trigger the situation. This very much depends on 
when the signal is going to be received by the process, and it can't really be 
controlled.

3. This slightly changes 1 line of code in `NativeProcessFreeBSD`, but I don't 
have the means to test if it even compiles, though the logic should be 
equivalent.

4. When I looked at `NativeProcessFreeBSD`, it seems that software single 
stepping isn't even being set up there, just checked for when the process 
stops. As I can see from history, software single stepping with similar code 
was in the "legacy FreeBSD plugin" which was removed here: 
https://reviews.llvm.org/D96555 , and then single stepping with shared code was 
added to the new plugin here: https://reviews.llvm.org/D95802 . Because I 
couldn't find where the `SetupSoftwareSingleStepping` would be called for 
FreeBSD I didn't change that, but this probably should be addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

What is the practical impact of the bug you are fixing?

I guess it is something like if you set signal handling info, then attach to 
something, that info is not used. Until you set it again, then it'll be sent 
and used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144390

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

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

There's one thing that's not clear to me about this patch. What is the 
relationship between `qGetTLSAddr`, as implemented here, and its implementation 
in GDB? I guess they're not compatible since we're not sending or using the 
offset and link map fields, but it's not clear to me whether we're at least 
implementing some subset of gdb functionality (e.g., what would gdbserver do if 
it received one of the packets that we're sending here). If we're not, then I 
don't think we should be using the same packet for this purpose.




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:186-193
-
-if (!SetRendezvousBreakpoint()) {
-  // If we cannot establish rendezvous breakpoint right now we'll try again
-  // at entry point.
-  ProbeEntry();
-}
-

How is this related to the rest of the patch?



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:196
   addr_t base_addr,
-  bool base_addr_is_offset) {
+  bool base_addr_is_offset) {  
   
   m_loaded_modules[module] = link_map_addr;

revert whitespace



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:731
 
-  addr_t link_map = it->second;
-  if (link_map == LLDB_INVALID_ADDRESS)
-return LLDB_INVALID_ADDRESS;
-
-  const DYLDRendezvous::ThreadInfo &metadata = m_rendezvous.GetThreadInfo();
-  if (!metadata.valid)
-return LLDB_INVALID_ADDRESS;
-
-  // Get the thread pointer.
-  addr_t tp = thread->GetThreadPointer();
-  if (tp == LLDB_INVALID_ADDRESS)
-return LLDB_INVALID_ADDRESS;
-
-  // Find the module's modid.
-  int modid_size = 4; // FIXME(spucci): This isn't right for big-endian 64-bit
-  int64_t modid = ReadUnsignedIntWithSizeInBytes(
-  link_map + metadata.modid_offset, modid_size);
-  if (modid == -1)
-return LLDB_INVALID_ADDRESS;
-
-  // Lookup the DTV structure for this thread.
-  addr_t dtv_ptr = tp + metadata.dtv_offset;
-  addr_t dtv = ReadPointer(dtv_ptr);
-  if (dtv == LLDB_INVALID_ADDRESS)
-return LLDB_INVALID_ADDRESS;
-
-  // Find the TLS block for this module.
-  addr_t dtv_slot = dtv + metadata.dtv_slot_size * modid;
-  addr_t tls_block = ReadPointer(dtv_slot + metadata.tls_offset);
-
-  Log *log = GetLog(LLDBLog::DynamicLoader);
-  LLDB_LOGF(log,
-"DynamicLoaderPOSIXDYLD::Performed TLS lookup: "
-"module=%s, link_map=0x%" PRIx64 ", tp=0x%" PRIx64
-", modid=%" PRId64 ", tls_block=0x%" PRIx64 "\n",
-module_sp->GetObjectName().AsCString(""), link_map, tp,
-(int64_t)modid, tls_block);
-
-  if (tls_block == LLDB_INVALID_ADDRESS)
-return LLDB_INVALID_ADDRESS;
-  else
-return tls_block + tls_file_addr;
+   addr_t link_map = it->second;
+   if (link_map == LLDB_INVALID_ADDRESS)

I'm not sure what these marks are (tabs maybe), but could you figure out how to 
revert them?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp:114-117
+  tp = LLDB_INVALID_ADDRESS;
+  Status error;
+  return error;
+}

Is this necessary, given that the function is already stubbed out in 
`NativeRegisterContext`?



Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h:85
 
+  virtual Status ReadThreadPointer(uint64_t &tp) override;
+

Return `llvm::Expected` perhaps?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:379
+  errno = 0;
+  auto ret= ptrace(static_cast<__ptrace_request>(PTRACE_ARCH_PRCTL), 
m_thread.GetID(),
+   &tp, (void *)ARCH_GET_FS, 0);

Any reason to not use the `PtraceWrapper` function?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2125-2126
+  lldb::tid_t tid =  StringExtractor(argv[0]).GetU64(LLDB_INVALID_ADDRESS, 16);
+  lldb::addr_t offset = StringExtractor(argv[1]).GetU64(LLDB_INVALID_ADDRESS, 
16);
+  lldb::addr_t lm = StringExtractor(argv[2]).GetU64(LLDB_INVALID_ADDRESS, 16);
+  (void) offset;

Might as well not bother parsing them, if they're going to be thrown away 
anyway.



Comment at: lldb/source/Target/Thread.cpp:1651-1653
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;

Could this be implemented in ThreadGDBRemote directly (without going through 
the GDBRemoteRegisterContext)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143698

___
lldb-commits mailing l

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: jingham, labath.
labath requested changes to this revision.
labath added a reviewer: jingham.
labath added a comment.
This revision now requires changes to proceed.

I'm afraid you're fixing this at the wrong end. This kind of complex thread 
control does not belong inside the debug stub.

IIUC, the problematic sequence of events is:

1. lldb sends a `vCont:s` packet
2. lldb-server resumes (PTRACE_SINGLESTEP)
3. process immediatelly stops again (without stepping) due to a pending signal
4. lldb-server decides to inject the signal and resumes (steps) again
5. process stops inside the signal handler
6. confusion ensues

If that's the case, then I think the bug is at step 4, specifically in this 
code:

  // Check if debugger should stop at this signal or just ignore it and resume
  // the inferior.
  if (m_signals_to_ignore.contains(signo)) {
 ResumeThread(thread, thread.GetState(), signo);
 return;
  }

I believe we should not be attempting to inject the signal if the thread was 
stepping. I think we should change this to report the signal to the client and 
let it figure out what to do. I.e., change this code into something like:

  if (m_signals_to_ignore.contains(signo) && thread.GetState() == 
eStateRunning) {
 ResumeThread(thread, eStateRunning, signo);
 return;
  }
  // else report the signal as usual

If that is not enough to fix the bug, then we can figure out what needs to be 
done on the client. @jingham might have something to say about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D142926#4124297 , @bulbazord wrote:

> In D142926#4123492 , @labath wrote:
>
>> Well... right now, there isn't anything else to do there.
>>
>> Anyway, I don't think this is particularly important. I don't know even if 
>> the distros would like to do this or whether to they'd prefer to ship 
>> unchanged headers. However, to me, it seems like the choice of the 
>> distribution method (framework vs. "traditional" unix layout) should be 
>> orthogonal to the choice of deswig-ifying the headers.
>
> I think I agree with you about the choice of distribution method being 
> orthogonal to choice of removing the swig ifdefs from the headers. That being 
> said, I don't know if any other distributors would care for this and I'm not 
> sure who I would ask. I added support for using `unifdef` in the 
> LLDB.framework case because I can ensure it is used and tested. I am hesitant 
> to add support without somebody to test/use this.
>
> Sort of tangential to this, the only other target where I think this would be 
> used would be the `lldb-headers` target. That seems to install **all** the 
> lldb headers, including all the private lldb headers. I could use `unifdef` 
> there (assuming it's available). Is it intended that it installs all the 
> private lldb headers though?

I don't know... maybe it isn't? I guess this isn't particularly important and 
people can and deswigification steps there if they want to.

I really like the rest of the patch though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D144224: [lldb] Extend SWIG SBProcess interface with WriteMemoryAsCString method

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/bindings/interface/SBProcessExtensions.i:11
+'''
+if not str[-1] == '\0':
+str += '\0'

I think this will fail if the string is empty (`""`). 

Somewhat relatedly, what if this appended the nul byte unconditionally (so it 
kinda behaved like std::string, which is always nul-terminated, but the nul 
byte is not counted as the length of the string)?


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

https://reviews.llvm.org/D144224

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

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Pavel Labath via Phabricator via lldb-commits]

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

Nice catch.

As discussed internally, it would be great if we could remove this 
android-specific code path (by removing support for OS versions which 
necessitated it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

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

[Lldb-commits] [lldb] bf874eb - [lldb] Use llvm::rotr (NFC)

[Kazu Hirata via lldb-commits]

Author: Kazu Hirata
Date: 2023-02-20T10:38:18-08:00
New Revision: bf874eb09bf3fb9ff13b6a06ec653acc5c041af0

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

LOG: [lldb] Use llvm::rotr (NFC)

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/ARMUtils.h 
b/lldb/source/Plugins/Process/Utility/ARMUtils.h
index a7aaa5ac7a1ff..9256f926275b8 100644
--- a/lldb/source/Plugins/Process/Utility/ARMUtils.h
+++ b/lldb/source/Plugins/Process/Utility/ARMUtils.h
@@ -11,6 +11,7 @@
 
 #include "ARMDefines.h"
 #include "InstructionUtils.h"
+#include "llvm/ADT/bit.h"
 #include "llvm/Support/MathExtras.h"
 
 // Common utilities for the ARM/Thumb Instruction Set Architecture.
@@ -173,8 +174,7 @@ static inline uint32_t ROR_C(const uint32_t value, const 
uint32_t amount,
 return 0;
   }
   *success = true;
-  uint32_t amt = amount % 32;
-  uint32_t result = Rotr32(value, amt);
+  uint32_t result = llvm::rotr(value, amount);
   carry_out = Bit32(value, 31);
   return result;
 }



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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3896
+  if (D->hasAttrs())
+ToField->setAttrs(D->getAttrs());
   ToField->setAccess(D->getAccess());

balazske wrote:
> The import of attributes is handled in function `ASTImporter::Import(Decl*)`. 
> This new line will probably copy all attributes, that may not work in all 
> cases dependent on the attribute types. This may interfere with the later 
> import of attributes, probably these will be duplicated. What was the need 
> for this line? (Simple attributes that do not have references to other nodes 
> could be copied at this place.)
Unfortunately it is too late to copy attribute in `ASTImporter::Import(Decl*)`, 
because field has already been added to record in a call to `ImportImpl 
(VisitFieldDecl/addDeclInternal)`. I've reused the current way of cloning 
attributes in `VisitFieldDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D144366: [RISCV]Add more pattern for fma ins

[Craig Topper via Phabricator via lldb-commits]

craig.topper added a comment.

I posted https://reviews.llvm.org/D17 to get these in the frontend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144366

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