[Lldb-commits] [PATCH] D117004: [lldb] Don't print "Command Options Usage:" for an alias with no options

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I also think this function is pretty confusing to read.

Enough that I decided not to attempt to unpick it for this bug! I also wondered 
how the quit command actually worked so thanks for solving that mystery.

I'll go with this as is but if I get a chance I'll try to simplify things like 
you suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117004

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

[Lldb-commits] [PATCH] D117004: [lldb] Don't print "Command Options Usage:" for an alias with no options

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG091e760cd398: [lldb] Don't print "Command Options 
Usage:" for an alias with no options (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117004

Files:
  lldb/source/Interpreter/Options.cpp
  lldb/test/API/commands/platform/basic/TestPlatformCommand.py


Index: lldb/test/API/commands/platform/basic/TestPlatformCommand.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -20,10 +20,12 @@
 self.runCmd("help platform")
 
 @no_debug_info_test
-def test_help_platform(self):
+def test_help_shell_alias(self):
 self.expect("help shell", substrs=["Run a shell command on the host.",
-   "shell "])
-
+   "shell ",
+   "'shell' is an abbreviation"])
+# "platform shell" has options. The "shell" alias for it does not.
+self.expect("help shell", substrs=["Command Options:"], matching=False)
 
 @no_debug_info_test
 def test_list(self):
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -403,7 +403,12 @@
   } else
 name = "";
 
-  strm.PutCString("\nCommand Options Usage:\n");
+  const uint32_t num_options = NumCommandOptions();
+  if (num_options == 0)
+return;
+
+  if (!only_print_args)
+strm.PutCString("\nCommand Options Usage:\n");
 
   strm.IndentMore(2);
 
@@ -413,10 +418,6 @@
   //   [options-for-level-1]
   //   etc.
 
-  const uint32_t num_options = NumCommandOptions();
-  if (num_options == 0)
-return;
-
   uint32_t num_option_sets = GetRequiredOptions().size();
 
   uint32_t i;
@@ -531,9 +532,9 @@
 strm << " " << arguments_str.GetString();
   }
 
-  strm.Printf("\n\n");
-
   if (!only_print_args) {
+strm.Printf("\n\n");
+
 // Now print out all the detailed information about the various options:
 // long form, short form and help text:
 //   -short  ( --long_name  )


Index: lldb/test/API/commands/platform/basic/TestPlatformCommand.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -20,10 +20,12 @@
 self.runCmd("help platform")
 
 @no_debug_info_test
-def test_help_platform(self):
+def test_help_shell_alias(self):
 self.expect("help shell", substrs=["Run a shell command on the host.",
-   "shell "])
-
+   "shell ",
+   "'shell' is an abbreviation"])
+# "platform shell" has options. The "shell" alias for it does not.
+self.expect("help shell", substrs=["Command Options:"], matching=False)
 
 @no_debug_info_test
 def test_list(self):
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -403,7 +403,12 @@
   } else
 name = "";
 
-  strm.PutCString("\nCommand Options Usage:\n");
+  const uint32_t num_options = NumCommandOptions();
+  if (num_options == 0)
+return;
+
+  if (!only_print_args)
+strm.PutCString("\nCommand Options Usage:\n");
 
   strm.IndentMore(2);
 
@@ -413,10 +418,6 @@
   //   [options-for-level-1]
   //   etc.
 
-  const uint32_t num_options = NumCommandOptions();
-  if (num_options == 0)
-return;
-
   uint32_t num_option_sets = GetRequiredOptions().size();
 
   uint32_t i;
@@ -531,9 +532,9 @@
 strm << " " << arguments_str.GetString();
   }
 
-  strm.Printf("\n\n");
-
   if (!only_print_args) {
+strm.Printf("\n\n");
+
 // Now print out all the detailed information about the various options:
 // long form, short form and help text:
 //   -short  ( --long_name  )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 091e760 - [lldb] Don't print "Command Options Usage:" for an alias with no options

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-01-12T10:07:38Z
New Revision: 091e760cd39848f3c41900b5104b3d92028d91b4

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

LOG: [lldb] Don't print "Command Options Usage:" for an alias with no options

"shell" is an alias to "platform shell -h --". Previously you would get this
help text:

(lldb) help shell
Run a shell command on the host.  Expects 'raw' input (see 'help raw-input'.)

Syntax: shell 

Command Options Usage:

'shell' is an abbreviation for 'platform shell -h   --'

Since the code doesn't handle the base command having options
but the alias removing them. With these changes you get:

(lldb) help shell
Run a shell command on the host.  Expects 'raw' input (see 'help raw-input'.)

Syntax: shell 

'shell' is an abbreviation for 'platform shell -h   --'

Note that we already handle a non-alias command having no options,
for example "quit":

(lldb) help quit
Quit the LLDB debugger.

Syntax: quit [exit-code]

Reviewed By: JDevlieghere, jingham

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

Added: 


Modified: 
lldb/source/Interpreter/Options.cpp
lldb/test/API/commands/platform/basic/TestPlatformCommand.py

Removed: 




diff  --git a/lldb/source/Interpreter/Options.cpp 
b/lldb/source/Interpreter/Options.cpp
index 4aa298f0382b1..feebe338bc9aa 100644
--- a/lldb/source/Interpreter/Options.cpp
+++ b/lldb/source/Interpreter/Options.cpp
@@ -403,7 +403,12 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject *cmd,
   } else
 name = "";
 
-  strm.PutCString("\nCommand Options Usage:\n");
+  const uint32_t num_options = NumCommandOptions();
+  if (num_options == 0)
+return;
+
+  if (!only_print_args)
+strm.PutCString("\nCommand Options Usage:\n");
 
   strm.IndentMore(2);
 
@@ -413,10 +418,6 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject *cmd,
   //   [options-for-level-1]
   //   etc.
 
-  const uint32_t num_options = NumCommandOptions();
-  if (num_options == 0)
-return;
-
   uint32_t num_option_sets = GetRequiredOptions().size();
 
   uint32_t i;
@@ -531,9 +532,9 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject *cmd,
 strm << " " << arguments_str.GetString();
   }
 
-  strm.Printf("\n\n");
-
   if (!only_print_args) {
+strm.Printf("\n\n");
+
 // Now print out all the detailed information about the various options:
 // long form, short form and help text:
 //   -short  ( --long_name  )

diff  --git a/lldb/test/API/commands/platform/basic/TestPlatformCommand.py 
b/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
index dc1701258246a..1c981e4bddd92 100644
--- a/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ b/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -20,10 +20,12 @@ def test_help_platform(self):
 self.runCmd("help platform")
 
 @no_debug_info_test
-def test_help_platform(self):
+def test_help_shell_alias(self):
 self.expect("help shell", substrs=["Run a shell command on the host.",
-   "shell "])
-
+   "shell ",
+   "'shell' is an abbreviation"])
+# "platform shell" has options. The "shell" alias for it does not.
+self.expect("help shell", substrs=["Command Options:"], matching=False)
 
 @no_debug_info_test
 def test_list(self):



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

[Lldb-commits] [PATCH] D112825: [lldb] Add MemoryTagMap class

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112825

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

[Lldb-commits] [PATCH] D112824: [lldb][AArch64] Add MakeTaggedRanges to MemoryTagManager

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 2 inline comments as done.
DavidSpickett added a comment.

ping!




Comment at: 
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:160
+  // we must use an untagged address.
+  MemoryRegionInfo::RangeType range(RemoveNonAddressBits(addr), len);
+  range = ExpandToGranule(range);

DavidSpickett wrote:
> DavidSpickett wrote:
> > omjavaid wrote:
> > > RemoveNonAddressBits hard-coded but symbols may resolve to higher order 
> > > bits containing PACs. For now I only came across code pointers with PACs. 
> > > But if you suspect code regions can be inputs to this function then may 
> > > be make accommodating changes probably in separate patch.
> > The end goal here is "memory read <...> --show-tags" and that command will 
> > have to use the ABI plugin in any case so the addresses this gets will be 
> > PAC/TBI/MTE free already.
> > 
> > So for now this should be addressed by https://reviews.llvm.org/D103626. 
> > (once I convince myself which address should be show in the output but 
> > that's not for this issue)
> > 
> > I agree that there is a bit of a conflict here with the ABI plugin removing 
> > everything vs the MemoryTagManager removing only the tag. However I think 
> > that it will work fine in this case.
> Speaking of conflict, `MemoryTagManagerAArch64MTE::RemoveNonAddressBits` 
> should probably be renamed to `RemoveTagBits` but I'll do that as another 
> change.
On second thought this will be better done in a general effort to remove the 
layering weirdness we currently have. Where we've got a tag manager that is 
removing the tags and the rest of the top byte. Then the ABI removing all of 
that and more.

Ideally the tag manager should only know about tags but some places assume it 
can also remove non-address bits in general. So this is more than just a rename 
and will take a bit more time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112824

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

[Lldb-commits] [PATCH] D107140: [lldb] Add option to show memory tags in memory read output

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107140

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

[Lldb-commits] [PATCH] D117065: [lldb/Plugins] Fix ScriptedInterface object ptr use-after-free

[Pavel Labath via Phabricator via lldb-commits]

labath requested changes to this revision.
labath added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp:54
+  m_object_instance = static_cast(
+  new StructuredPythonObject(ret_val));
 

This doesn't sound right. This object (`StructuredPythonObject` instance) is 
definitely not created by python and will now be leaked. If I correctly 
understand the problem, the issue is that the this object gets a non-owning 
reference (the `ret_val` argument) to the underlying python object, and then 
frees it as if it was owning it. If that's the case, then the solution is to 
INCREF it in the constructor (or switch to using a PythonObject wrapper, which 
will then handle the lifetime management.

You may also be interested in D114722 (which I hope to update soon). It's not 
_directly_ related to this, but it touches the same parts of the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117065

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

[Lldb-commits] [PATCH] D117076: [lldb/Plugins] Fix ScriptedThread IndexID reporting

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It seems to me like the ScriptedThread class would benefit from using the 
fallible constructor idiom. If you move all the potentially failing operations 
(initialization of the ScriptedThreadInterface instance?) into a (public) 
`static Expected Create(...)` method, then the 
constructor will not need the funky by-ref error argument, and it will be able 
to access the thread id sufficiently early.


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

https://reviews.llvm.org/D117076

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

[Lldb-commits] [lldb] 3fd9c90 - [lldb][AArch64] Correct top nibble setting in memory tag read tests

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-01-12T12:28:13Z
New Revision: 3fd9c90bdc04df451d9bb348450b5ad424c822c6

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

LOG: [lldb][AArch64] Correct top nibble setting in memory tag read tests

Due to a missing cast the << 60 always resulted in zero leaving
the top nibble empty. So we weren't actually testing that lldb
ignores those bits in addition to the tag bits.

Correct that and also set the top nibbles to ascending values
so that we can catch if lldb only removes one of the tag bits
and top nibble, but not both.

In future the tag manager will likely only remove the tag bits
and leave non-address bits to the ABI plugin but for now make
sure we're testing what we claim to implement.

Added: 


Modified: 

lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
lldb/test/API/linux/aarch64/mte_tag_access/main.c

Removed: 




diff  --git 
a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
 
b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
index b0ce9c1f55c44..29c19aa68fb72 100644
--- 
a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
+++ 
b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
@@ -122,10 +122,11 @@ def test_mte_tag_read(self):
   "\[0x[0-9A-Fa-f]+f0, 0x[0-9A-Fa-f]+00\): 0xf 
\(mismatch\)\n"
   "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 
\(mismatch\)$"])
 
-# Tags in start/end are ignored when creating the range.
-# So this is not an error despite start/end having 
diff erent tags
-self.expect("memory tag read mte_buf mte_buf_alt_tag+16",
-patterns=["Logical tag: 0x9\n"
+# Top byte is ignored when creating the range, not just the 4 tag bits.
+# So even though these two pointers have 
diff erent top bytes
+# and the start's is > the end's, this is not an error.
+self.expect("memory tag read mte_buf_alt_tag mte_buf+16",
+patterns=["Logical tag: 0xa\n"
   "Allocation tags:\n"
   "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 
\(mismatch\)$"])
 

diff  --git a/lldb/test/API/linux/aarch64/mte_tag_access/main.c 
b/lldb/test/API/linux/aarch64/mte_tag_access/main.c
index ecafe3ae7bd34..493733d09562c 100644
--- a/lldb/test/API/linux/aarch64/mte_tag_access/main.c
+++ b/lldb/test/API/linux/aarch64/mte_tag_access/main.c
@@ -67,16 +67,20 @@ int main(int argc, char const *argv[]) {
 
   // Tag the original pointer with 9
   mte_buf = __arm_mte_create_random_tag(mte_buf, ~(1 << 9));
-  // A 
diff erent tag so that buf_alt_tag > buf if you don't handle the tag
+  // A 
diff erent tag so that mte_buf_alt_tag > mte_buf if you don't handle the
+  // tag
   char *mte_buf_alt_tag = __arm_mte_create_random_tag(mte_buf, ~(1 << 10));
 
   // lldb should be removing the whole top byte, not just the tags.
   // So fill 63-60 with something non zero so we'll fail if we only remove 
tags.
-#define SET_TOP_NIBBLE(ptr) (char *)((size_t)(ptr) | (0xA << 60))
-  mte_buf = SET_TOP_NIBBLE(mte_buf);
-  mte_buf_alt_tag = SET_TOP_NIBBLE(mte_buf_alt_tag);
-  mte_buf_2 = SET_TOP_NIBBLE(mte_buf_2);
-  mte_read_only = SET_TOP_NIBBLE(mte_read_only);
+#define SET_TOP_NIBBLE(ptr, value) 
\
+  (char *)((size_t)(ptr) | ((size_t)((value)&0xf) << 60))
+  // mte_buf_alt_tag's nibble > mte_buf to check that lldb isn't just removing
+  // tag bits but the whole top byte when making ranges.
+  mte_buf = SET_TOP_NIBBLE(mte_buf, 0xA);
+  mte_buf_alt_tag = SET_TOP_NIBBLE(mte_buf_alt_tag, 0xB);
+  mte_buf_2 = SET_TOP_NIBBLE(mte_buf_2, 0xC);
+  mte_read_only = SET_TOP_NIBBLE(mte_read_only, 0xD);
 
   // Breakpoint here
   return 0;



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

[Lldb-commits] [PATCH] D117103: [lldb] [Process/FreeBSD] Set current thread ID on events

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

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

Set the current thread ID to the thread where an event happened.
As a result, e.g. when a signal is delivered to a thread other than
the first one, the respective T packet refers to the signaled thread
rather than the first thread (with no stop reason).  While this doesn't
strictly make a difference to the LLDB client, some of the lldb-server
tests rely on that for convenience.


https://reviews.llvm.org/D117103

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -254,6 +254,7 @@
 
 for (const auto &thread : m_threads)
   static_cast(*thread).SetStoppedByExec();
+SetCurrentThreadID(m_threads.front()->GetID());
 SetState(StateType::eStateStopped, true);
 return;
   }
@@ -312,6 +313,7 @@
 } else
   thread->SetStoppedByBreakpoint();
 FixupBreakpointPCAsNeeded(*thread);
+SetCurrentThreadID(thread->GetID());
   }
   SetState(StateType::eStateStopped, true);
   return;
@@ -333,11 +335,13 @@
 if (wp_index != LLDB_INVALID_INDEX32) {
   regctx.ClearWatchpointHit(wp_index);
   thread->SetStoppedByWatchpoint(wp_index);
+  SetCurrentThreadID(thread->GetID());
   SetState(StateType::eStateStopped, true);
   break;
 }
 
 thread->SetStoppedByTrace();
+SetCurrentThreadID(thread->GetID());
   }
 
   SetState(StateType::eStateStopped, true);
@@ -370,9 +374,10 @@
 static_cast(*abs_thread);
 assert(info.pl_lwpid >= 0);
 if (info.pl_lwpid == 0 ||
-static_cast(info.pl_lwpid) == thread.GetID())
+static_cast(info.pl_lwpid) == thread.GetID()) {
   thread.SetStoppedBySignal(info.pl_siginfo.si_signo, &info.pl_siginfo);
-else
+  SetCurrentThreadID(thread.GetID());
+} else
   thread.SetStoppedWithNoReason();
   }
   SetState(StateType::eStateStopped, true);
@@ -809,6 +814,9 @@
   break;
 }
   }
+
+  if (GetCurrentThreadID() == thread_id)
+SetCurrentThreadID(m_threads.front()->GetID());
 }
 
 Status NativeProcessFreeBSD::Attach() {


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -254,6 +254,7 @@
 
 for (const auto &thread : m_threads)
   static_cast(*thread).SetStoppedByExec();
+SetCurrentThreadID(m_threads.front()->GetID());
 SetState(StateType::eStateStopped, true);
 return;
   }
@@ -312,6 +313,7 @@
 } else
   thread->SetStoppedByBreakpoint();
 FixupBreakpointPCAsNeeded(*thread);
+SetCurrentThreadID(thread->GetID());
   }
   SetState(StateType::eStateStopped, true);
   return;
@@ -333,11 +335,13 @@
 if (wp_index != LLDB_INVALID_INDEX32) {
   regctx.ClearWatchpointHit(wp_index);
   thread->SetStoppedByWatchpoint(wp_index);
+  SetCurrentThreadID(thread->GetID());
   SetState(StateType::eStateStopped, true);
   break;
 }
 
 thread->SetStoppedByTrace();
+SetCurrentThreadID(thread->GetID());
   }
 
   SetState(StateType::eStateStopped, true);
@@ -370,9 +374,10 @@
 static_cast(*abs_thread);
 assert(info.pl_lwpid >= 0);
 if (info.pl_lwpid == 0 ||
-static_cast(info.pl_lwpid) == thread.GetID())
+static_cast(info.pl_lwpid) == thread.GetID()) {
   thread.SetStoppedBySignal(info.pl_siginfo.si_signo, &info.pl_siginfo);
-else
+  SetCurrentThreadID(thread.GetID());
+} else
   thread.SetStoppedWithNoReason();
   }
   SetState(StateType::eStateStopped, true);
@@ -809,6 +814,9 @@
   break;
 }
   }
+
+  if (GetCurrentThreadID() == thread_id)
+SetCurrentThreadID(m_threads.front()->GetID());
 }
 
 Status NativeProcessFreeBSD::Attach() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117103: [lldb] [Process/FreeBSD] Set current thread ID on events

[Pavel Labath via Phabricator via lldb-commits]

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

LG, though I wouldn't say this is just "convenience". I'd say this is the 
expected behaviour, and it just happens that the lldb client does not mind if 
we do not do that.


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

https://reviews.llvm.org/D117103

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

[Lldb-commits] [PATCH] D114722: [LLDB] Fix Python GIL-not-held issues

[Pavel Labath via Phabricator via lldb-commits]

labath commandeered this revision.
labath edited reviewers, added: rupprecht; removed: labath.
labath added a reviewer: mib.
labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:277-285
+  if (_Py_IsFinalizing()) {
+// Leak m_py_obj rather than crashing the process.
+// https://docs.python.org/3/c-api/init.html#c.PyGILState_Ensure
+  } else {
+auto gstate = PyGILState_Ensure();
+Py_DECREF(m_py_obj);
+PyGILState_Release(gstate);

labath wrote:
> As I mentioned internally, I think this should be placed inside the 
> ScriptInterpreterPythonImpl destructor (and elsewhere, if needed), as that's 
> the level at which we do our normal locking. There it can become a regular 
> `Locker` object, since that function will be called from 
> SBDebugger::Terminate (or maybe even earlier, in either case it will be 
> before any global destructors run).
> 
> The reason this can't be done with StructuredPythonObject, is because this 
> one gets passed on to python code, which can delete it (or not) at pretty 
> much arbitrary time. PythonObjects OTOH, are just C++  handles to python 
> objects, and we should be able to keep track of their lifetimes reasonably 
> well.
> 
> We can put an assert here to ensure the callers obey this contract.
It turns out this is not as easy as I made it sound. I thought one could just 
put the lock around the body of the destructor, but (of course) destructors for 
subobjects run only after the body of the main object desctructor terminates. I 
mean, it would still be possible to make it work that way, but it wouldn't be 
as pretty. So, after some soul-searching, I decided to keep the current 
implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114722

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

[Lldb-commits] [PATCH] D114722: [LLDB] Fix Python GIL-not-held issues

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 399302.
labath added a comment.

This just fixes the style issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114722

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1413,14 +1413,9 @@
   if (class_name == nullptr || class_name[0] == '\0')
 return StructuredData::GenericSP();
 
-  void *ret_val;
-
-  {
-Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN,
-   Locker::FreeLock);
-ret_val = LLDBSWIGPython_CreateFrameRecognizer(class_name,
-   m_dictionary_name.c_str());
-  }
+  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
+  void *ret_val = LLDBSWIGPython_CreateFrameRecognizer(
+  class_name, m_dictionary_name.c_str());
 
   return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
 }
@@ -1477,14 +1472,9 @@
   if (!process_sp)
 return StructuredData::GenericSP();
 
-  void *ret_val;
-
-  {
-Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN,
-   Locker::FreeLock);
-ret_val = LLDBSWIGPythonCreateOSPlugin(
-class_name, m_dictionary_name.c_str(), process_sp);
-  }
+  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
+  void *ret_val = LLDBSWIGPythonCreateOSPlugin(
+  class_name, m_dictionary_name.c_str(), process_sp);
 
   return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
 }
@@ -1732,17 +1722,13 @@
   if (!python_interpreter)
 return {};
 
-  void *ret_val;
-
-  {
-Locker py_lock(this,
-   Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
-ret_val = LLDBSwigPythonCreateScriptedThreadPlan(
-class_name, python_interpreter->m_dictionary_name.c_str(),
-args_data, error_str, thread_plan_sp);
-if (!ret_val)
-  return {};
-  }
+  Locker py_lock(this,
+ Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
+  void *ret_val = LLDBSwigPythonCreateScriptedThreadPlan(
+  class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
+  error_str, thread_plan_sp);
+  if (!ret_val)
+return {};
 
   return StructuredData::ObjectSP(new StructuredPythonObject(ret_val));
 }
@@ -1835,16 +1821,12 @@
   if (!python_interpreter)
 return StructuredData::GenericSP();
 
-  void *ret_val;
-
-  {
-Locker py_lock(this,
-   Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
+  Locker py_lock(this,
+ Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
 
-ret_val = LLDBSwigPythonCreateScriptedBreakpointResolver(
-class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
-bkpt_sp);
-  }
+  void *ret_val = LLDBSwigPythonCreateScriptedBreakpointResolver(
+  class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
+  bkpt_sp);
 
   return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
 }
@@ -1910,16 +1892,12 @@
 return StructuredData::GenericSP();
   }
 
-  void *ret_val;
-
-  {
-Locker py_lock(this,
-   Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
+  Locker py_lock(this,
+ Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
 
-ret_val = LLDBSwigPythonCreateScriptedStopHook(
-target_sp, class_name, python_interpreter->m_dictionary_name.c_str(),
-args_data, error);
-  }
+  void *ret_val = LLDBSwigPythonCreateScriptedStopHook(
+  target_sp, class_name, python_interpreter->m_dictionary_name.c_str(),
+  args_data, error);
 
   return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
 }
@@ -2010,14 +1988,10 @@
   if (!python_interpreter)
 return StructuredData::ObjectSP();
 
-  void *ret_val = nullptr;
-
-  {
-Locker py_lock(this,
-   Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
-ret_val = LLDBSwigPythonCreateSyntheticProvider(
-class_name, python_interpreter->m_dictionary_name.c_str(), valobj);
-  }
+  Locker py_lock(this,
+ Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
+  void *ret_val = LLDBSwigPythonCreateSyntheticProvider(
+  class_name, python_interpreter->m_dictionary_name.c_str(), valobj);
 
   return StructuredData::ObjectSP(new StructuredPythonObject(ret_val));
 }
@@ -2032,14 +2006,10 @@
   i

[Lldb-commits] [PATCH] D117065: [lldb/Plugins] Fix ScriptedInterface object ptr use-after-free

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp:54
+  m_object_instance = static_cast(
+  new StructuredPythonObject(ret_val));
 

labath wrote:
> This doesn't sound right. This object (`StructuredPythonObject` instance) is 
> definitely not created by python and will now be leaked. If I correctly 
> understand the problem, the issue is that the this object gets a non-owning 
> reference (the `ret_val` argument) to the underlying python object, and then 
> frees it as if it was owning it. If that's the case, then the solution is to 
> INCREF it in the constructor (or switch to using a PythonObject wrapper, 
> which will then handle the lifetime management.
> 
> You may also be interested in D114722 (which I hope to update soon). It's not 
> _directly_ related to this, but it touches the same parts of the code.
So, as far as I can tell `ret_val` is an owned reference (in 
`LLDBSwigPythonCreateScriptedThread`, it comes from `PythonObject.release()`). 
Could it be that something else is freeing (decreffing) the object more times 
than it should (thereby releasing the references that are supposed to be held 
here) and this code gets blamed/crashes just because it happens to run last?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117065

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

[Lldb-commits] [PATCH] D115662: [lldb][DWARF] Remove duplicate DIE type assignment

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am sorry for the delay. This is quite messy so it took me a while to form an 
opinion on this. I think option one would be the best choice. Doing it there 
means the these would get set in a central place (that is still fairly near the 
place where the type is being constructed). The main thing which makes this 
confusing is the name, and if we change that to something like FinalizeType, 
then it would look natural.

As an alternative (though I don't know if this would work), we could make a 
utility function that constructs a type and does all of these finalization 
steps. That way it would be impossible to forget to add a type to the list, or 
to add it twice. (I know that there can be multiple DIEs referring to the same 
type, so it's fine if some extra DIEToType lines remain, but we shouldn't need 
to set the symbol context scope, or add to the type list twice).

Would you be interested in trying that out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115662

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

[Lldb-commits] [PATCH] D117113: [lldb] [llgs] Implement qXfer:siginfo:read

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Implement the qXfer:siginfo:read that is used to read the siginfo_t
(extended signal information) for the current thread.  This is currently
implemented on FreeBSD and Linux.


https://reviews.llvm.org/D117113

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  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/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -12,6 +12,7 @@
 
 import binascii
 import itertools
+import struct
 
 import unittest2
 import gdbremote_testcase
@@ -993,6 +994,13 @@
 self.assertEqual(supported_dict.get('qXfer:libraries-svr4:read', '-'),
  expected)
 
+def test_qSupported_siginfo_read(self):
+expected = ('+' if lldbplatformutil.getPlatform()
+in ["freebsd", "linux"] else '-')
+supported_dict = self.get_qSupported_dict()
+self.assertEqual(supported_dict.get('qXfer:siginfo:read', '-'),
+ expected)
+
 def test_qSupported_QPassSignals(self):
 expected = ('+' if lldbplatformutil.getPlatform()
 in ["freebsd", "linux", "netbsd"] else '-')
@@ -1374,3 +1382,81 @@
 True)
 context = self.expect_gdbremote_sequence()
 self.assertEqual(context["O_content"], b"test\r\na=z\r\na*}#z\r\n")
+
+@skipUnlessPlatform(oslist=["freebsd", "linux"])
+@add_test_categories(["llgs"])
+def test_qXfer_siginfo_read(self):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault", "thread:new", "sleep:10"])
+self.test_sequence.add_log_lines(["read packet: $c#63"], True)
+self.expect_gdbremote_sequence()
+
+# Run until SIGSEGV comes in.
+self.reset_test_sequence()
+# TODO: freebsd doesn't report crashing thread in T
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);",
+  "capture": {1: "signo", 2: "thread_id"},
+  }], True)
+
+# Figure out which thread crashed.
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+self.assertEqual(int(context["signo"], 16),
+ lldbutil.get_signal_number('SIGSEGV'))
+crashing_thread = int(context["thread_id"], 16)
+
+# Grab siginfo for the crashing thread.
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $Hg{:x}#00".format(crashing_thread),
+ "send packet: $OK#00",
+ "read packet: $qXfer:siginfo:read::0,80:#00",
+ {"direction": "send",
+  "regex": re.compile(r"^\$([^E])(.*)#[0-9a-fA-F]{2}$",
+  re.MULTILINE | re.DOTALL),
+  "capture": {1: "response_type", 2: "content_raw"},
+  }], True)
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Ensure we end up with all data in one packet.
+self.assertEqual(context.get("response_type"), "l")
+
+# Decode binary data.
+content_raw = context.get("content_raw")
+self.assertIsNotNone(content_raw)
+content = self.decode_gdbremote_binary(content_raw).encode("latin1")
+
+# Decode siginfo_t.
+platform = lldbplatformutil.getPlatform()
+pad = ""
+if sys.maxsize > 2**32:
+pad = "i"
+signo_idx = 0
+errno_idx = 1
+code_idx = 2
+addr_idx = -1
+SEGV_MAPERR = 1
+if platform == "linux":
+# si_signo, si_errno, si_code, [pad], _sifields._sigfault.si_addr
+format_str = "iii{}P".format(pad)
+elif platform == "freebsd":
+# si_signo, si_errno, si_code, si_pid, si_uid, si_status, si_addr
+format_str = "iiP"
+elif platform == "netbsd":
+# _signo, _code, _errno, [pad], _reason._fault._addr
+format_str = "iii{}P".format(pad)
+errno_idx = 2
+code_idx = 1
+
+decoder = struct.Struct(format_str)
+decoded = deco

[Lldb-commits] [lldb] 9a1ce35 - [lldb] [Process/FreeBSD] Set current thread ID on events

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-01-12T15:40:13+01:00
New Revision: 9a1ce35d7e7fdfa6d45fb24abc3ad30e8eb6c6f4

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

LOG: [lldb] [Process/FreeBSD] Set current thread ID on events

Set the current thread ID to the thread where an event happened.
As a result, e.g. when a signal is delivered to a thread other than
the first one, the respective T packet refers to the signaled thread
rather than the first thread (with no stop reason).  While this doesn't
strictly make a difference to the LLDB client, it is the expected
behavior.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index a62d3c1ba0522..7290d300a41f5 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -254,6 +254,7 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
 
 for (const auto &thread : m_threads)
   static_cast(*thread).SetStoppedByExec();
+SetCurrentThreadID(m_threads.front()->GetID());
 SetState(StateType::eStateStopped, true);
 return;
   }
@@ -312,6 +313,7 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
 } else
   thread->SetStoppedByBreakpoint();
 FixupBreakpointPCAsNeeded(*thread);
+SetCurrentThreadID(thread->GetID());
   }
   SetState(StateType::eStateStopped, true);
   return;
@@ -333,11 +335,13 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t 
pid) {
 if (wp_index != LLDB_INVALID_INDEX32) {
   regctx.ClearWatchpointHit(wp_index);
   thread->SetStoppedByWatchpoint(wp_index);
+  SetCurrentThreadID(thread->GetID());
   SetState(StateType::eStateStopped, true);
   break;
 }
 
 thread->SetStoppedByTrace();
+SetCurrentThreadID(thread->GetID());
   }
 
   SetState(StateType::eStateStopped, true);
@@ -370,9 +374,10 @@ void NativeProcessFreeBSD::MonitorSignal(lldb::pid_t pid, 
int signal) {
 static_cast(*abs_thread);
 assert(info.pl_lwpid >= 0);
 if (info.pl_lwpid == 0 ||
-static_cast(info.pl_lwpid) == thread.GetID())
+static_cast(info.pl_lwpid) == thread.GetID()) {
   thread.SetStoppedBySignal(info.pl_siginfo.si_signo, &info.pl_siginfo);
-else
+  SetCurrentThreadID(thread.GetID());
+} else
   thread.SetStoppedWithNoReason();
   }
   SetState(StateType::eStateStopped, true);
@@ -809,6 +814,9 @@ void NativeProcessFreeBSD::RemoveThread(lldb::tid_t 
thread_id) {
   break;
 }
   }
+
+  if (GetCurrentThreadID() == thread_id)
+SetCurrentThreadID(m_threads.front()->GetID());
 }
 
 Status NativeProcessFreeBSD::Attach() {



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

[Lldb-commits] [PATCH] D117103: [lldb] [Process/FreeBSD] Set current thread ID on events

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a1ce35d7e7f: [lldb] [Process/FreeBSD] Set current thread ID 
on events (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117103

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -254,6 +254,7 @@
 
 for (const auto &thread : m_threads)
   static_cast(*thread).SetStoppedByExec();
+SetCurrentThreadID(m_threads.front()->GetID());
 SetState(StateType::eStateStopped, true);
 return;
   }
@@ -312,6 +313,7 @@
 } else
   thread->SetStoppedByBreakpoint();
 FixupBreakpointPCAsNeeded(*thread);
+SetCurrentThreadID(thread->GetID());
   }
   SetState(StateType::eStateStopped, true);
   return;
@@ -333,11 +335,13 @@
 if (wp_index != LLDB_INVALID_INDEX32) {
   regctx.ClearWatchpointHit(wp_index);
   thread->SetStoppedByWatchpoint(wp_index);
+  SetCurrentThreadID(thread->GetID());
   SetState(StateType::eStateStopped, true);
   break;
 }
 
 thread->SetStoppedByTrace();
+SetCurrentThreadID(thread->GetID());
   }
 
   SetState(StateType::eStateStopped, true);
@@ -370,9 +374,10 @@
 static_cast(*abs_thread);
 assert(info.pl_lwpid >= 0);
 if (info.pl_lwpid == 0 ||
-static_cast(info.pl_lwpid) == thread.GetID())
+static_cast(info.pl_lwpid) == thread.GetID()) {
   thread.SetStoppedBySignal(info.pl_siginfo.si_signo, &info.pl_siginfo);
-else
+  SetCurrentThreadID(thread.GetID());
+} else
   thread.SetStoppedWithNoReason();
   }
   SetState(StateType::eStateStopped, true);
@@ -809,6 +814,9 @@
   break;
 }
   }
+
+  if (GetCurrentThreadID() == thread_id)
+SetCurrentThreadID(m_threads.front()->GetID());
 }
 
 Status NativeProcessFreeBSD::Attach() {


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -254,6 +254,7 @@
 
 for (const auto &thread : m_threads)
   static_cast(*thread).SetStoppedByExec();
+SetCurrentThreadID(m_threads.front()->GetID());
 SetState(StateType::eStateStopped, true);
 return;
   }
@@ -312,6 +313,7 @@
 } else
   thread->SetStoppedByBreakpoint();
 FixupBreakpointPCAsNeeded(*thread);
+SetCurrentThreadID(thread->GetID());
   }
   SetState(StateType::eStateStopped, true);
   return;
@@ -333,11 +335,13 @@
 if (wp_index != LLDB_INVALID_INDEX32) {
   regctx.ClearWatchpointHit(wp_index);
   thread->SetStoppedByWatchpoint(wp_index);
+  SetCurrentThreadID(thread->GetID());
   SetState(StateType::eStateStopped, true);
   break;
 }
 
 thread->SetStoppedByTrace();
+SetCurrentThreadID(thread->GetID());
   }
 
   SetState(StateType::eStateStopped, true);
@@ -370,9 +374,10 @@
 static_cast(*abs_thread);
 assert(info.pl_lwpid >= 0);
 if (info.pl_lwpid == 0 ||
-static_cast(info.pl_lwpid) == thread.GetID())
+static_cast(info.pl_lwpid) == thread.GetID()) {
   thread.SetStoppedBySignal(info.pl_siginfo.si_signo, &info.pl_siginfo);
-else
+  SetCurrentThreadID(thread.GetID());
+} else
   thread.SetStoppedWithNoReason();
   }
   SetState(StateType::eStateStopped, true);
@@ -809,6 +814,9 @@
   break;
 }
   }
+
+  if (GetCurrentThreadID() == thread_id)
+SetCurrentThreadID(m_threads.front()->GetID());
 }
 
 Status NativeProcessFreeBSD::Attach() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a subscriber: labath.
mib added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:332
+lldb::ThreadSP thread_sp =
+std::make_shared(*this, error, *val->GetAsGeneric());
+

@labath This is why I switched to a raw pointer in D117065. Basically, I fetch 
a python dictionary for the `thread_info_sp`. It gets converted to a 
`StructuredData::Dictionary` and every element gets re-allocated in 
`pythonObject::CreateStructuredDictionary`. Then I iterate over each of element 
of the StructuredData::Dictionary and passes the StructuredData::Object to the 
ScriptedThread constructor.

The ScriptedThread needs a valid reference to the python object instance to 
perform calls on it. However, when the structured dictionary goes out of scope 
(at the end of the function), all the objects it holds get destroyed so when 
the ScriptedThread tries to make a call on the python object is causes a 
heap-use-after-free.


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

https://reviews.llvm.org/D117071

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

[Lldb-commits] [PATCH] D117113: [lldb] [llgs] Implement qXfer:siginfo:read

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:195-198
+  virtual llvm::Expected>
+  GetSiginfo() const {
+return llvm::make_error();
+  }

I think it would be more natural/consistent to have this function on a specific 
NativeThreadProtocol object (and let the user choose whether it wants the 
siginfo for the current thread, or someone else).



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1987
+NativeProcessLinux::GetSiginfo() const {
+  siginfo_t siginfo;
+  Status error = GetSignalInfo(GetCurrentThreadID(), &siginfo);

I'd be better to outright allocate a memory buffer of the right size 
(`getNewUninitMemBuffer`), and directly write the data there.



Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1398
+self.reset_test_sequence()
+# TODO: freebsd doesn't report crashing thread in T
+self.test_sequence.add_log_lines(

Didn't D117103 fix that?



Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1437
+pad = ""
+if sys.maxsize > 2**32:
+pad = "i"

you want the address width of the debugged binary, not python, right? We 
already have some utilities to help with that (grep for 
`parse_process_info_response`).


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

https://reviews.llvm.org/D117113

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

[Lldb-commits] [lldb] 10bc336 - Revert "[LLDB][NativePDB] Add support for inlined functions"

[Stella Stamenova via lldb-commits]

Author: Stella Stamenova
Date: 2022-01-12T08:53:19-08:00
New Revision: 10bc3362a1a8a3df2660bf65db0ec1ccab646e1b

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

LOG: Revert "[LLDB][NativePDB] Add support for inlined functions"

This reverts commit 945aa520ef07a3edb655f3f38e4c3023658dd623.

This commit broke the windows lldb bot.

Added: 


Modified: 
lldb/include/lldb/Symbol/LineTable.h
lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

Removed: 
lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.lldbinit
lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s



diff  --git a/lldb/include/lldb/Symbol/LineTable.h 
b/lldb/include/lldb/Symbol/LineTable.h
index b5121b29fe028..5cd22bd831eef 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -206,6 +206,7 @@ class LineTable {
 
   LineTable *LinkLineTable(const FileRangeMap &file_range_map);
 
+protected:
   struct Entry {
 Entry()
 : line(0), is_start_of_statement(false), 
is_start_of_basic_block(false),
@@ -302,7 +303,6 @@ class LineTable {
 uint16_t file_idx = 0;
   };
 
-protected:
   struct EntrySearchInfo {
 LineTable *line_table;
 lldb_private::Section *a_section;

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
index d8f737612c257..9f09c0accc876 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
@@ -106,24 +106,6 @@ static void ParseExtendedInfo(PdbIndex &index, 
CompilandIndexItem &item) {
   }
 }
 
-static void ParseInlineeLineTableForCompileUnit(CompilandIndexItem &item) {
-  for (const auto &ss : item.m_debug_stream.getSubsectionsArray()) {
-if (ss.kind() != DebugSubsectionKind::InlineeLines)
-  continue;
-
-DebugInlineeLinesSubsectionRef inlinee_lines;
-llvm::BinaryStreamReader reader(ss.getRecordData());
-if (llvm::Error error = inlinee_lines.initialize(reader)) {
-  consumeError(std::move(error));
-  continue;
-}
-
-for (const InlineeSourceLine &Line : inlinee_lines) {
-  item.m_inline_map[Line.Header->Inlinee] = Line;
-}
-  }
-}
-
 CompilandIndexItem::CompilandIndexItem(
 PdbCompilandId id, llvm::pdb::ModuleDebugStreamRef debug_stream,
 llvm::pdb::DbiModuleDescriptor descriptor)
@@ -160,7 +142,6 @@ CompilandIndexItem 
&CompileUnitIndex::GetOrCreateCompiland(uint16_t modi) {
   cci = std::make_unique(
   PdbCompilandId{modi}, std::move(debug_stream), std::move(descriptor));
   ParseExtendedInfo(m_index, *cci);
-  ParseInlineeLineTableForCompileUnit(*cci);
 
   cci->m_strings.initialize(debug_stream.getSubsectionsArray());
   PDBStringTable &strings = cantFail(m_index.pdb().getStringTable());

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h 
b/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
index 4ad27de986954..088de970cbf32 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
@@ -9,12 +9,10 @@
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_COMPILEUNITINDEX_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_COMPILEUNITINDEX_H
 
-#include "lldb/Utility/RangeMap.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/DebugInfo/CodeView/DebugInlineeLinesSubsection.h"
 #include "llvm/DebugInfo/CodeView/StringsAndChecksums.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
@@ -71,17 +69,6 @@ struct CompilandIndexItem {
   // command line, etc.  This usually contains exactly 5 items which
   // are references to other strings.
   llvm::SmallVector m_build_info;
-
-  // Inlinee lines table in this compile unit.
-  std::map
-  m_inline_map;
-
-  // It's the line table parsed from DEBUG_S_LINES sections, mapping the file
-  // address range to file index and source line number.
-  using GlobalLineTable =
-  lldb_private::RangeDataVector>;
-  GlobalLineTable m_global_line_table;
 };
 
 /// Indexes information about all compile units.  This is really just a map of

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 578b369945d01..e859b1d5a86cf 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/

[Lldb-commits] [lldb] 8fec756 - [lldb] Disable one more watchpoint test on Windows

[Stella Stamenova via lldb-commits]

Author: Stella Stamenova
Date: 2022-01-12T08:55:48-08:00
New Revision: 8fec756c0b54f8720b746b103ac277b5a153dff7

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

LOG: [lldb] Disable one more watchpoint test on Windows

This should be the last remaining flaky watchpoint test on Windows

Added: 


Modified: 

lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py

Removed: 




diff  --git 
a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
 
b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
index 542a5d811064e..d6d8cca2f439f 100644
--- 
a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
+++ 
b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
@@ -18,6 +18,7 @@ class WatchpointForMultipleThreadsTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 main_spec = lldb.SBFileSpec("main.cpp", False)
 
+@skipIfWindows # This test is flaky on Windows
 def test_watchpoint_before_thread_start(self):
 """Test that we can hit a watchpoint we set before starting another 
thread"""
 self.do_watchpoint_test("Before running the thread")



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

[Lldb-commits] [PATCH] D104886: [lldb] Fix that the embedded Python REPL crashes if it receives SIGINT

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 399390.

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

https://reviews.llvm.org/D104886

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py

Index: lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py
===
--- /dev/null
+++ lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py
@@ -0,0 +1,75 @@
+"""
+Test sending SIGINT to the embedded Python REPL.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestCase(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def start_python_repl(self):
+""" Starts up the embedded Python REPL."""
+self.launch()
+# Start the embedded Python REPL via the 'script' command.
+self.child.send("script -l python --\n")
+# Wait for the Python REPL prompt.
+self.child.expect(">>>")
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfWindows
+def test_while_evaluating_code(self):
+""" Tests SIGINT handling while Python code is being evaluated."""
+self.start_python_repl()
+
+# Start a long-running command that we try to abort with SIGINT.
+# Note that we dont actually wait 1s in this code as pexpect or
+# lit will kill the test way before that.
+self.child.send("import time; print('running' + 'now'); time.sleep(1);\n")
+
+# Make sure the command is actually being evaluated at the moment by
+# looking at the string that the command is printing.
+# Don't check for a needle that also occurs in the program itself to
+# prevent that echoing will make this check pass unintentionally.
+self.child.expect("runningnow")
+
+# Send SIGINT to the LLDB process.
+self.child.sendintr()
+
+# This should get transformed to a KeyboardInterrupt which is the same
+# behaviour as the standalone Python REPL. It should also interrupt
+# the evaluation of our sleep statement.
+self.child.expect("KeyboardInterrupt")
+# Send EOF to quit the Python REPL.
+self.child.sendeof()
+
+self.quit()
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+# FIXME: On Linux the Python code that reads from stdin seems to block until
+# it has finished reading a line before handling any queued signals.
+@skipIfLinux
+@skipIfWindows
+def test_while_waiting_on_input(self):
+""" Tests SIGINT handling while the REPL is waiting on input from
+stdin."""
+self.start_python_repl()
+
+# Send SIGINT to the LLDB process.
+self.child.sendintr()
+# This should get transformed to a KeyboardInterrupt which is the same
+# behaviour as the standalone Python REPL.
+self.child.expect("KeyboardInterrupt")
+# Send EOF to quit the Python REPL.
+self.child.sendeof()
+
+self.quit()
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -70,6 +70,14 @@
 #define LLDBSwigPyInit init_lldb
 #endif
 
+#if defined(_WIN32)
+// Don't mess with the signal handlers on Windows.
+#define LLDB_USE_PYTHON_SET_INTERRUPT 0
+#else
+// PyErr_SetInterrupt was introduced in 3.2.
+#define LLDB_USE_PYTHON_SET_INTERRUPT  \
+  (PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 2) || (PY_MAJOR_VERSION > 3)
+#endif
 
 static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger &debugger) {
   ScriptInterpreter *script_interpreter =
@@ -920,6 +928,22 @@
 }
 
 bool ScriptInterpreterPythonImpl::Interrupt() {
+#if LLDB_USE_PYTHON_SET_INTERRUPT
+  // If the interpreter isn't evaluating any Python at the moment then return
+  // false to signal that this function didn't handle the interrupt and the
+  // next component should try handling it.
+  if (!IsExecutingPython())
+return false;
+
+  // Tell Python that it should pretend to have received a SIGINT.
+  PyErr_SetInterrupt();
+  // PyErr_SetInterrupt has no way to return an error so we can only pretend the
+  // signal got successfully handled and return true.
+  // Python 3.10 introduces PyErr_SetInterruptEx that could return an error, but
+  // the error handling is limited to checking the arguments which would be
+  // just our (hardcoded) input signal code SIGINT, so that's not use

[Lldb-commits] [PATCH] D114722: [LLDB] Fix Python GIL-not-held issues

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:277-285
+  if (_Py_IsFinalizing()) {
+// Leak m_py_obj rather than crashing the process.
+// https://docs.python.org/3/c-api/init.html#c.PyGILState_Ensure
+  } else {
+auto gstate = PyGILState_Ensure();
+Py_DECREF(m_py_obj);
+PyGILState_Release(gstate);

labath wrote:
> labath wrote:
> > As I mentioned internally, I think this should be placed inside the 
> > ScriptInterpreterPythonImpl destructor (and elsewhere, if needed), as 
> > that's the level at which we do our normal locking. There it can become a 
> > regular `Locker` object, since that function will be called from 
> > SBDebugger::Terminate (or maybe even earlier, in either case it will be 
> > before any global destructors run).
> > 
> > The reason this can't be done with StructuredPythonObject, is because this 
> > one gets passed on to python code, which can delete it (or not) at pretty 
> > much arbitrary time. PythonObjects OTOH, are just C++  handles to python 
> > objects, and we should be able to keep track of their lifetimes reasonably 
> > well.
> > 
> > We can put an assert here to ensure the callers obey this contract.
> It turns out this is not as easy as I made it sound. I thought one could just 
> put the lock around the body of the destructor, but (of course) destructors 
> for subobjects run only after the body of the main object desctructor 
> terminates. I mean, it would still be possible to make it work that way, but 
> it wouldn't be as pretty. So, after some soul-searching, I decided to keep 
> the current implementation.
Makes sense!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114722

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

[Lldb-commits] [PATCH] D117065: [lldb/Plugins] Fix ScriptedInterface object ptr use-after-free

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp:54
+  m_object_instance = static_cast(
+  new StructuredPythonObject(ret_val));
 

labath wrote:
> labath wrote:
> > This doesn't sound right. This object (`StructuredPythonObject` instance) 
> > is definitely not created by python and will now be leaked. If I correctly 
> > understand the problem, the issue is that the this object gets a non-owning 
> > reference (the `ret_val` argument) to the underlying python object, and 
> > then frees it as if it was owning it. If that's the case, then the solution 
> > is to INCREF it in the constructor (or switch to using a PythonObject 
> > wrapper, which will then handle the lifetime management.
> > 
> > You may also be interested in D114722 (which I hope to update soon). It's 
> > not _directly_ related to this, but it touches the same parts of the code.
> So, as far as I can tell `ret_val` is an owned reference (in 
> `LLDBSwigPythonCreateScriptedThread`, it comes from 
> `PythonObject.release()`). Could it be that something else is freeing 
> (decreffing) the object more times than it should (thereby releasing the 
> references that are supposed to be held here) and this code gets 
> blamed/crashes just because it happens to run last?
Pavel said it better than I would have :-) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117065

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

[Lldb-commits] [PATCH] D117068: [lldb/Plugins] Add ScriptedProcess::GetThreadsInfo interface

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Looks straightforward. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117068

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

[Lldb-commits] [PATCH] D117070: [lldb/Plugins] Move ScriptedThreadInterface to ScriptedThread

[Jonas Devlieghere via Phabricator via lldb-commits]

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

I was slightly confused by the description, but IIUC this moves the interface 
into the scripted thread instance. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117070

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

[Lldb-commits] [PATCH] D117139: [lldb] Copy ScriptedInterface object instance to a new StructuredData object

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch adds the ability to pass a pointer of a script object instance
to initialize a ScriptedInterface instead of having the call the
script object initializer in the ScriptedInterface constructor.

rdar://87425859

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117139

Files:
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -24,7 +24,8 @@
 
   StructuredData::GenericSP
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
- StructuredData::DictionarySP args_sp) override;
+ StructuredData::DictionarySP args_sp,
+ void *instance_obj = nullptr) override;
 
   lldb::tid_t GetThreadID() override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
@@ -31,7 +31,7 @@
 
 StructuredData::GenericSP ScriptedThreadPythonInterface::CreatePluginObject(
 const llvm::StringRef class_name, ExecutionContext &exe_ctx,
-StructuredData::DictionarySP args_sp) {
+StructuredData::DictionarySP args_sp, void *instance_obj) {
 
   if (class_name.empty())
 return {};
@@ -43,15 +43,17 @@
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
  Locker::FreeLock);
 
-  void *ret_val = LLDBSwigPythonCreateScriptedThread(
-  class_name.str().c_str(), m_interpreter.GetDictionaryName(), process_sp,
-  args_impl, error_string);
+  if (!instance_obj) {
+instance_obj = LLDBSwigPythonCreateScriptedThread(
+class_name.str().c_str(), m_interpreter.GetDictionaryName(), process_sp,
+args_impl, error_string);
 
-  if (!ret_val)
-return {};
+if (!instance_obj)
+  return {};
+  }
 
   m_object_instance_sp =
-  StructuredData::GenericSP(new StructuredPythonObject(ret_val));
+  StructuredData::GenericSP(new StructuredPythonObject(instance_obj));
 
   return m_object_instance_sp;
 }
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
@@ -25,7 +25,8 @@
   StructuredData::GenericSP
   CreatePluginObject(const llvm::StringRef class_name,
  ExecutionContext &exe_ctx,
- StructuredData::DictionarySP args_sp) override;
+ StructuredData::DictionarySP args_sp,
+ void *instance_obj = nullptr) override;
 
   Status Launch() override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -32,7 +32,7 @@
 
 StructuredData::GenericSP ScriptedProcessPythonInterface::CreatePluginObject(
 llvm::StringRef class_name, ExecutionContext &exe_ctx,
-StructuredData::DictionarySP args_sp) {
+StructuredData::DictionarySP args_sp, void *instance_obj) {
   if (class_name.empty())
 return {};
 
@@ -43,15 +43,17 @@
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
  Locker::FreeLock);
 
-  void *ret_val = LLDBSwigPythonCreateScriptedProcess(
-  class_name.str().c_str(), m_interpreter.GetDictionaryName(), target_sp,
-  args_impl, error_string);
+  if (!instance_obj) {
+instance_obj = LLDBSwigPythonCreateScriptedProcess(
+class_name.str().c_str(), m_interpreter.GetDictionaryName(), target_sp,
+args_impl, error_string);
 
-  if (!ret_val)
-return {};
+if (!instance_obj)
+  return {};
+  }
 
   m_object_instance_sp =
-

[Lldb-commits] [PATCH] D117065: [lldb/Plugins] Fix ScriptedInterface object ptr use-after-free

[Med Ismail Bennani via Phabricator via lldb-commits]

mib abandoned this revision.
mib added a comment.

Abandoning this in favor of D117139 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117065

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

[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 399413.
mib added a reviewer: labath.
mib added a comment.

Update after abandoning D117065  for D117139 
.

To create the ScriptedThread, we pass  a `StructuredData::Object*` that 
contains a borrowed reference of the python ScriptedThread instance, to the 
constructor.

Previously, we just re-assigned the ScriptedThread StructuredData script object 
shared_ptr with the `StructuredData::Object*`.
Now, we "extract" the python borrowed reference from the 
`StructuredData::Object*` and create a new StructuredData object with it, to 
make sure we can access it even after the `StructuredData::Object*` gets freed 
when exiting `ScriptedProcess::DoUpdateThreadList `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117071

Files:
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/test/API/functionalities/scripted_process/Makefile
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/test/API/functionalities/scripted_process/main.cpp
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -1,4 +1,4 @@
-import os,struct,signal
+import os,json,struct,signal
 
 from typing import Any, Dict
 
@@ -21,6 +21,14 @@
 idx = int(self.backing_target_idx.GetStringValue(100))
 self.corefile_target = target.GetDebugger().GetTargetAtIndex(idx)
 self.corefile_process = self.corefile_target.GetProcess()
+for corefile_thread in self.corefile_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"backing_target_idx" : idx,
+"thread_idx" : corefile_thread.GetIndexID()
+}))
+
+self.threads[corefile_thread.GetThreadID()] = StackCoreScriptedThread(self, structured_data)
 
 def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
 mem_region = lldb.SBMemoryRegionInfo()
@@ -70,23 +78,43 @@
 class StackCoreScriptedThread(ScriptedThread):
 def __init__(self, process, args):
 super().__init__(process, args)
-self.backing_target_idx = args.GetValueForKey("backing_target_idx")
+backing_target_idx = args.GetValueForKey("backing_target_idx")
+thread_idx = args.GetValueForKey("thread_idx")
+
+def extract_value_from_structured_data(data, default_val):
+if data and data.IsValid():
+if data.GetType() == lldb.eStructuredDataTypeInteger:
+return data.GetIntegerValue(default_val)
+if data.GetType() == lldb.eStructuredDataTypeString:
+return int(data.GetStringValue(100))
+return None
+
+#TODO: Change to Walrus operator (:=) with oneline if assignment
+# Requires python 3.8
+val = extract_value_from_structured_data(thread_idx, 0)
+if val is not None:
+self.idx = val
 
 self.corefile_target = None
 self.corefile_process = None
-if (self.backing_target_idx and self.backing_target_idx.IsValid()):
-if self.backing_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
-idx = self.backing_target_idx.GetIntegerValue(42)
-if self.backing_target_idx.GetType() == lldb.eStructuredDataTypeString:
-idx = int(self.backing_target_idx.GetStringValue(100))
-self.corefile_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+self.corefile_thread = None
+
+#TODO: Change to Walrus operator (:=) with oneline if assignment
+# Requires python 3.8
+val = extract_value_from_structured_data(backing_target_idx, 42)
+if val is not None:
+self.corefile_target = self.target.GetDebugger().GetTargetAtIndex(val)
 self.corefile_process = self.corefile_target.GetProcess()
+self.corefile_thread = self.corefile_process.GetThreadByIndexID(self.idx)
+
+if self.corefile_thread:
+self.id = self.corefile_thread.GetThreadID()
 
 def get_thread_id(self) -> int:
-return 0x19
+return self.id
 
 def get_name(se

[Lldb-commits] [lldb] 1f53dd1 - [CODE OWNERS] Add wallace as code owner

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-01-12T12:36:30-08:00
New Revision: 1f53dd1f23870a27123c50f4fe955e9c31a19634

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

LOG: [CODE OWNERS] Add wallace as code owner

Added: 


Modified: 
lldb/CODE_OWNERS.txt

Removed: 




diff  --git a/lldb/CODE_OWNERS.txt b/lldb/CODE_OWNERS.txt
index 1af1061c545a1..ae679e90d9243 100644
--- a/lldb/CODE_OWNERS.txt
+++ b/lldb/CODE_OWNERS.txt
@@ -10,7 +10,7 @@ beautification by scripts.  The fields are: name (N), email 
(E), web-address
 
 N: Greg Clayton
 E: clayb...@gmail.com
-D: Overall LLDB architecture, Host (common+macosx), Symbol, API, ABI, 
Mac-specific code, 
+D: Overall LLDB architecture, Host (common+macosx), Symbol, API, ABI, 
Mac-specific code,
 D: DynamicLoader, ObjectFile, IOHandler, EditLine, Core/Value*, Watchpoints, 
debugserver
 D: Build scripts, Test suite, Platform, gdb-remote, Anything not covered by 
this file
 
@@ -27,7 +27,7 @@ D: FreeBSD
 
 N: Jason Molenda
 E: jmole...@apple.com
-D: ABI, Disassembler, Unwinding, iOS, debugserver, Platform, ObjectFile, 
SymbolFile, 
+D: ABI, Disassembler, Unwinding, iOS, debugserver, Platform, ObjectFile, 
SymbolFile,
 D: SymbolVendor, DWARF, gdb-remote
 
 N: Kamil Rytarowski
@@ -42,3 +42,8 @@ D: Test suite
 N: Pavel Labath
 E: lab...@google.com
 D: Linux, Android
+
+N: Walter Erquinigo
+E: a20012...@gmail.com
+E: walterme...@fb.com
+D: Trace, TraceCursor, TraceExport, intel-pt, lldb-vscode, Data Formatters



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

[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 399426.

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

https://reviews.llvm.org/D117071

Files:
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/test/API/functionalities/scripted_process/Makefile
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/test/API/functionalities/scripted_process/main.cpp
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -1,4 +1,4 @@
-import os,struct,signal
+import os,json,struct,signal
 
 from typing import Any, Dict
 
@@ -21,6 +21,14 @@
 idx = int(self.backing_target_idx.GetStringValue(100))
 self.corefile_target = target.GetDebugger().GetTargetAtIndex(idx)
 self.corefile_process = self.corefile_target.GetProcess()
+for corefile_thread in self.corefile_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"backing_target_idx" : idx,
+"thread_idx" : corefile_thread.GetIndexID()
+}))
+
+self.threads[corefile_thread.GetThreadID()] = StackCoreScriptedThread(self, structured_data)
 
 def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
 mem_region = lldb.SBMemoryRegionInfo()
@@ -70,23 +78,43 @@
 class StackCoreScriptedThread(ScriptedThread):
 def __init__(self, process, args):
 super().__init__(process, args)
-self.backing_target_idx = args.GetValueForKey("backing_target_idx")
+backing_target_idx = args.GetValueForKey("backing_target_idx")
+thread_idx = args.GetValueForKey("thread_idx")
+
+def extract_value_from_structured_data(data, default_val):
+if data and data.IsValid():
+if data.GetType() == lldb.eStructuredDataTypeInteger:
+return data.GetIntegerValue(default_val)
+if data.GetType() == lldb.eStructuredDataTypeString:
+return int(data.GetStringValue(100))
+return None
+
+#TODO: Change to Walrus operator (:=) with oneline if assignment
+# Requires python 3.8
+val = extract_value_from_structured_data(thread_idx, 0)
+if val is not None:
+self.idx = val
 
 self.corefile_target = None
 self.corefile_process = None
-if (self.backing_target_idx and self.backing_target_idx.IsValid()):
-if self.backing_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
-idx = self.backing_target_idx.GetIntegerValue(42)
-if self.backing_target_idx.GetType() == lldb.eStructuredDataTypeString:
-idx = int(self.backing_target_idx.GetStringValue(100))
-self.corefile_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+self.corefile_thread = None
+
+#TODO: Change to Walrus operator (:=) with oneline if assignment
+# Requires python 3.8
+val = extract_value_from_structured_data(backing_target_idx, 42)
+if val is not None:
+self.corefile_target = self.target.GetDebugger().GetTargetAtIndex(val)
 self.corefile_process = self.corefile_target.GetProcess()
+self.corefile_thread = self.corefile_process.GetThreadByIndexID(self.idx)
+
+if self.corefile_thread:
+self.id = self.corefile_thread.GetThreadID()
 
 def get_thread_id(self) -> int:
-return 0x19
+return self.id
 
 def get_name(self) -> str:
-return StackCoreScriptedThread.__name__ + ".thread-1"
+return StackCoreScriptedThread.__name__ + ".thread-" + str(self.id)
 
 def get_stop_reason(self) -> Dict[str, Any]:
 return { "type": lldb.eStopReasonSignal, "data": {
@@ -109,10 +137,9 @@
 return self.frame_zero[0:0]
 
 def get_register_context(self) -> str:
-thread = self.corefile_process.GetSelectedThread()
-if not thread or thread.GetNumFrames() == 0:
+if not self.corefile_thread or self.corefile_thread.GetNumFrames() == 0:
 return None
-frame = thread.GetFrameAtIndex(0)
+frame = self.corefile_thread.GetFrameAtIndex(0)
 
 GPRs = None
 registerSet = frame.registers # Returns an SBVal

[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:51
+  if (script_object)
+instance_obj = script_object->GetValue();
+

Instead of re-assigning `m_script_object_sp` with `script_object` directly, we 
get the python object ptr and pass it to `CreatePluginObject` that will create 
a new `StructuredData::GenericSP` holding that ref.


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

https://reviews.llvm.org/D117071

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

[Lldb-commits] [PATCH] D112825: [lldb] Add MemoryTagMap class

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/MemoryTagMap.h:29
+  /// Non-null pointer to a memory tag manager.
+  MemoryTagMap(const MemoryTagManager *manager);
+

If the pointer should be non-null, then should this take a reference instead?



Comment at: lldb/include/lldb/Target/MemoryTagMap.h:46
+
+  bool empty() const;
+

I'm on the fence about this one. I personally prefer `empty()` for consistency 
with llvm and the standard library, but technically this should be `Empty()` to 
match the rest of LLDB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112825

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

[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Never thought I'd ask someone to merge two patches, but I think it might make 
reviewing easier if you merge D117139  into 
this patch. :-)

If I understand the patch correctly, you're getting the underlying `PyObject` 
out of the `PythonDateObject` when passing it to the scripted thread interface. 
I assume that works because the objects are retained by being stored in a dict 
on the Python side (referring to `threads` in ScriptedProcess, from D117068 
). Why can't we pass the PythonObject around 
instead? That seems simpler but more importantly would guarantee the underlying 
object remains alive (even if it weren't stored on the Python side)  by keeping 
the ref-count incremented.


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

https://reviews.llvm.org/D117071

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

[Lldb-commits] [PATCH] D117113: [lldb] [llgs] Implement qXfer:siginfo:read

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

mgorny updated this revision to Diff 399478.
mgorny marked 4 inline comments as done.
mgorny added a comment.

Implemented the requested changes.


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

https://reviews.llvm.org/D117113

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/common/NativeThreadProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.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/Linux/NativeThreadLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -12,6 +12,7 @@
 
 import binascii
 import itertools
+import struct
 
 import unittest2
 import gdbremote_testcase
@@ -993,6 +994,13 @@
 self.assertEqual(supported_dict.get('qXfer:libraries-svr4:read', '-'),
  expected)
 
+def test_qSupported_siginfo_read(self):
+expected = ('+' if lldbplatformutil.getPlatform()
+in ["freebsd", "linux"] else '-')
+supported_dict = self.get_qSupported_dict()
+self.assertEqual(supported_dict.get('qXfer:siginfo:read', '-'),
+ expected)
+
 def test_qSupported_QPassSignals(self):
 expected = ('+' if lldbplatformutil.getPlatform()
 in ["freebsd", "linux", "netbsd"] else '-')
@@ -1374,3 +1382,83 @@
 True)
 context = self.expect_gdbremote_sequence()
 self.assertEqual(context["O_content"], b"test\r\na=z\r\na*}#z\r\n")
+
+@skipUnlessPlatform(oslist=["freebsd", "linux"])
+@add_test_categories(["llgs"])
+def test_qXfer_siginfo_read(self):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault", "thread:new", "sleep:10"])
+self.test_sequence.add_log_lines(["read packet: $c#63"], True)
+self.expect_gdbremote_sequence()
+
+# Run until SIGSEGV comes in.
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);",
+  "capture": {1: "signo", 2: "thread_id"},
+  }], True)
+
+# Figure out which thread crashed.
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+self.assertEqual(int(context["signo"], 16),
+ lldbutil.get_signal_number('SIGSEGV'))
+crashing_thread = int(context["thread_id"], 16)
+
+# Grab siginfo for the crashing thread.
+self.reset_test_sequence()
+self.add_process_info_collection_packets()
+self.test_sequence.add_log_lines(
+["read packet: $Hg{:x}#00".format(crashing_thread),
+ "send packet: $OK#00",
+ "read packet: $qXfer:siginfo:read::0,80:#00",
+ {"direction": "send",
+  "regex": re.compile(r"^\$([^E])(.*)#[0-9a-fA-F]{2}$",
+  re.MULTILINE | re.DOTALL),
+  "capture": {1: "response_type", 2: "content_raw"},
+  }], True)
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Ensure we end up with all data in one packet.
+self.assertEqual(context.get("response_type"), "l")
+
+# Decode binary data.
+content_raw = context.get("content_raw")
+self.assertIsNotNone(content_raw)
+content = self.decode_gdbremote_binary(content_raw).encode("latin1")
+
+# Decode siginfo_t.
+process_info = self.parse_process_info_response(context)
+pad = ""
+if process_info["ptrsize"] == "8":
+pad = "i"
+signo_idx = 0
+errno_idx = 1
+code_idx = 2
+addr_idx = -1
+SEGV_MAPERR = 1
+if process_info["ostype"] == "linux":
+# si_signo, si_errno, si_code, [pad], _sifields._sigfault.si_addr
+format_str = "iii{}P".format(pad)
+elif process_info["ostype"].startswith("freebsd"):
+# si_signo, si_errno, si_code, si_pid, si_uid, si_status, si_addr
+format_str = "iiP"
+elif process_info["ostype"].startswith("netbsd"):
+# _signo, _code, _errno, [pad], _reason._fau

[Lldb-commits] [PATCH] D117113: [lldb] [llgs] Implement qXfer:siginfo:read

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

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1987
+NativeProcessLinux::GetSiginfo() const {
+  siginfo_t siginfo;
+  Status error = GetSignalInfo(GetCurrentThreadID(), &siginfo);

labath wrote:
> I'd be better to outright allocate a memory buffer of the right size 
> (`getNewUninitMemBuffer`), and directly write the data there.
I suppose there's no similar solution for  the FreeBSD case where the returned 
struct is a member of the temporary struct?



Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1398
+self.reset_test_sequence()
+# TODO: freebsd doesn't report crashing thread in T
+self.test_sequence.add_log_lines(

labath wrote:
> Didn't D117103 fix that?
Yeah, I left a comment, so I don't forget, then I've forgotten that I've left a 
comment ;-D.


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

https://reviews.llvm.org/D117113

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added a reviewer: jingham.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Convert the `crashlog` command to be implemented as a class. The `Symbolicate`
function is switched to a class, to implement `get_long_help`. The text for the
long help comes from the help output generated by `OptionParser`. That is, the
output of `help crashlog` is the same as `crashlog --help`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1226,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
 lldb.debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in the crash log and functions can
 be disassembled and lookups can be

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

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

The output of `help crashlog` is:

  (lldb) help crashlog 
  For more information run 'help crashlog'  Expects 'raw' input (see 'help 
raw-input'.)
  
  Syntax: crashlog
  Usage: crashlog [options]  [FILE ...]
  
  Symbolicate one or more darwin crash log files to provide source file and line
  information, inlined stack frames back to the concrete functions, and
  disassemble the location of the crash for the first frame of the crashed
  thread. If this script is imported into the LLDB command interpreter, a
  "crashlog" command will be added to the interpreter for use at the LLDB
  command line. After a crash log has been parsed and symbolicated, a target
  will have been created that has all of the shared libraries loaded at the load
  addresses found in the crash log file. This allows you to explore the program
  as if it were stopped at the locations described in the crash log and
  functions can be disassembled and lookups can be performed using the addresses
  found in the crash log.
  
  Options:
-h, --helpshow this help message and exit
-v, --verbose display verbose debug info
-g, --debug   display verbose debug logging
-a, --load-allload all executable images, not just the images found
  in the crashed stack frames
--images  show image list
--debug-delay=NSECpause for NSEC seconds for debugger
-c, --crashed-onlyonly symbolicate the crashed thread
-d DISASSEMBLE_DEPTH, --disasm-depth=DISASSEMBLE_DEPTH
  set the depth in stack frames that should be
  disassembled (default is 1)
-D, --disasm-all  enabled disassembly of frames on all threads (not just
  the crashed thread)
-B DISASSEMBLE_BEFORE, --disasm-before=DISASSEMBLE_BEFORE
  the number of instructions to disassemble before the
  frame PC
-A DISASSEMBLE_AFTER, --disasm-after=DISASSEMBLE_AFTER
  the number of instructions to disassemble after the
  frame PC
-C NLINES, --source-context=NLINES
  show NLINES source lines of source context (default =
  4)
--source-frames=NFRAMES
  show source for NFRAMES (default = 4)
--source-all  show source for all threads, not just the crashed
  thread
-i, --interactive parse all crash logs and enter interactive mode


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D115508: Reland "[lldb] Remove non address bits when looking up memory regions"

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

Apologies for late response!!!




Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1668
+if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS) ||
+(abi && (abi->FixDataAddress(load_addr) != load_addr))) {
   result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",

Perhaps this condition should only run when argument is not equal to 1. When 
user supplies an argument which has non-address-bits set, load_addr may not be 
equal to value returned by FixDataAddress(load_address). This will fire a 
invalid argument error which according to comment above we do not intend?





Comment at: lldb/source/Target/Process.cpp:5891
+  // is at or beyond the end of mappable memory.
+  !(abi && (abi->FixDataAddress(range_end) != range_end)));
 

Just to confirm this assumes range base will always be free of any 
non-address-bits. ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115508

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399517.
kastiglione added a comment.

refer to the debugger parameter, not lldb.debugger


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1226,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
-lldb.debugger.HandleCommand(
+
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/examples/python/crashlog.py:1236-1240
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')

Note that I introduced `__lldb_init_module` here because there's some bug when 
registering a command class using `lldb.debugger`. In other words, this failed:

```
lldb.debugger.HandleCommand('command script add -c mod.Command cmd')
```

and this succeeds:

```
def __lldb_init_module(debugger, _):
debugger.HandleCommand('command script add -c mod.Command cmd')
```

When using the first form, an error would happen when running the command. The 
error message is "no function to execute".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/examples/python/crashlog.py:1236-1240
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')

kastiglione wrote:
> Note that I introduced `__lldb_init_module` here because there's some bug 
> when registering a command class using `lldb.debugger`. In other words, this 
> failed:
> 
> ```
> lldb.debugger.HandleCommand('command script add -c mod.Command cmd')
> ```
> 
> and this succeeds:
> 
> ```
> def __lldb_init_module(debugger, _):
> debugger.HandleCommand('command script add -c mod.Command cmd')
> ```
> 
> When using the first form, an error would happen when running the command. 
> The error message is "no function to execute".
That makes sense.  All of the "lldb.{debugger, target, process, thread, etc}" 
variables are only set when entering the interactive script interpreter.  They 
don't get set when processing "command script import".  So lldb.debugger would 
have been None here.

In general, since pretty much no python we write in the lldb project is going 
to run in the interactive interpreter, you should never use those lldb.etc 
variables.

I actually think we could make the lldb.debugger work, since that really is a 
singleton for the command interpreter, but it seems much better form to pass 
debugger directly to whoever needs it, so for the sake of clarity they all get 
unset when we leave the interactive script interpreter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Oops, forgot to include the "why requesting changes..."

Can you see if also adding a "get_short_help" gets rid of that dopey:

  For more information run 'help crashlog'  Expects 'raw' input (see 'help 
raw-input'.)

line?  It's getting added as the placeholder text in CommandObjectPythonObject 
& CommandObjectPythonFunction.  It is not a very good place holder, but anyway 
having the short help is good for "apropos" listings and the like.  And if 
those classes are doing the right thing, and actual short help should override 
the placeholder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If adding the short help doesn't fix this, I'm fine with this change as is.  We 
can go down the getting rid of that placeholder rabbit hole at some other time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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