[Lldb-commits] [PATCH] D128453: Automate checking for "command that takes no arguments" being passed arguments in CommandObjectParsed

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: srhines.
labath added a comment.

In D128453#3606296 , @JDevlieghere 
wrote:

> This is great, it both guarantees consistently and enforces command objects 
> registering their arguments. LGTM.
>
> For the RenderScript plugin, I remember this at an LLVM social when @labath 
> was in town. At the time, we were very close to being able to get rid of it, 
> which is now several years ago. Pavel, do you remember who we spoke to and if 
> we've reached the point where this can go away?

I don't know what's the state of renderscript, but I think @srhines is the 
person you have in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128453

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

[Lldb-commits] [PATCH] D128504: debugserver: spawn process in its own process group

[Alessandro Arzilli via Phabricator via lldb-commits]

aarzilli created this revision.
aarzilli added reviewers: jasonmolenda, clayborg.
aarzilli added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
aarzilli requested review of this revision.
Herald added a subscriber: lldb-commits.

Change POSIX spawn launch flavor to spawn process in its own process group, 
like the fork/exec flavor does.
This is useful because the target process can then be made controlling process 
for its tty and receive terminal signals (such as SIGINT generated in response 
to the user pressing ^C) without debugserver also receiving them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128504

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -3268,7 +3268,7 @@
 return INVALID_NUB_PROCESS;
 
   flags = POSIX_SPAWN_START_SUSPENDED | POSIX_SPAWN_SETSIGDEF |
-  POSIX_SPAWN_SETSIGMASK;
+  POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETPGROUP;
   if (disable_aslr)
 flags |= _POSIX_SPAWN_DISABLE_ASLR;
 


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -3268,7 +3268,7 @@
 return INVALID_NUB_PROCESS;
 
   flags = POSIX_SPAWN_START_SUSPENDED | POSIX_SPAWN_SETSIGDEF |
-  POSIX_SPAWN_SETSIGMASK;
+  POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETPGROUP;
   if (disable_aslr)
 flags |= _POSIX_SPAWN_DISABLE_ASLR;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128221: [LLDB] Add Arm64 CodeView to LLDB regnum mapping

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D128221#3598132 , @DavidSpickett 
wrote:

> Are there public docs for these numbers?

There is no public doc but I have copied these mappings from 
https://github.com/microsoft/microsoft-pdb which was opensourced by microsoft. 
Also cross checked with mapping available at 
llvm/include/llvm/DebugInfo/CodeView/CodeViewRegisters.def


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

https://reviews.llvm.org/D128221

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

[Lldb-commits] [lldb] 91d61c1 - [LLDB] Mark TestExprsChar Xfail for Windows/AArch64

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2022-06-24T13:59:22+04:00
New Revision: 91d61c1431c2ec46fa7a243db1643154580ab043

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

LOG: [LLDB] Mark TestExprsChar Xfail for Windows/AArch64

test_unsigned_char test in TestExprsChar.py fails on AArch64/Windows
platform. There is known bug already present for the failure for various
arch/os combinations. This patch marks the test as xfail for
windows/AArch64.

Added: 


Modified: 
lldb/test/API/commands/expression/char/TestExprsChar.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/char/TestExprsChar.py 
b/lldb/test/API/commands/expression/char/TestExprsChar.py
index cd9c55b5a44bb..849615ef91453 100644
--- a/lldb/test/API/commands/expression/char/TestExprsChar.py
+++ b/lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -38,5 +38,6 @@ def test_signed_char(self):
 'arm64_32'],
 bugnumber="llvm.org/pr23069, ")
 @expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
+@expectedFailureAll(oslist=['windows'], archs=['aarch64'], 
bugnumber="llvm.org/pr23069")
 def test_unsigned_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-funsigned-char'})



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

[Lldb-commits] [PATCH] D128484: [NFC][lldb][trace] Rename trace session to trace bundle

[Jakob Johnson via Phabricator via lldb-commits]

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

lgtm - thanks for doing this renaming 🙂




Comment at: lldb/include/lldb/Core/PluginManager.h:346
 
-  static TraceCreateInstanceForSessionFile
+  static TraceCreateInstanceForBundle
   GetTraceCreateCallback(llvm::StringRef plugin_name);

nit:  `FromBundle` makes more sense than `ForBundle` imo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128484

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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Pavel Labath via Phabricator via lldb-commits]

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

In D128321#3605592 , @JDevlieghere 
wrote:

> I also considered that, but that just delays the problem until I want to 
> enable this handler for the always-on logging (as outlined in 
> https://discourse.llvm.org/c/subprojects/lldb/8). I haven't really looked 
> into where that logic would live, but the reproducers lived in Utility so I 
> was expecting all of this to be there as well.

I don't think that's a particularly big problem. Any code that's needed to 
enable always-on logging (to /a/ log handler) can stay in Utility. Only the 
code which actually enables it (and passes a specific log handler) needs to 
live outside. One way to do that would be to put something in the 
SystemInitializer class (like we did for reproducers). Another would be to use 
the shiny new global lldbinit file feature (which would allow site 
customization). :P




Comment at: lldb/source/Host/common/Host.cpp:110
+#if !defined(__APPLE__)
+void Host::SystemLog(const std::string &message) {
+  fprintf(stderr, "%s", message.c_str());

Why std::string? I'd expect StringRef (as that's what the LogHandler class 
uses), const char * (as that can avoid string copying sometimes) or a Twine (in 
case you want to be fancy), but a string seems like it combines the worst 
properties of all of these.


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Host/common/Host.cpp:110
+#if !defined(__APPLE__)
+void Host::SystemLog(const std::string &message) {
+  fprintf(stderr, "%s", message.c_str());

labath wrote:
> Why std::string? I'd expect StringRef (as that's what the LogHandler class 
> uses), const char * (as that can avoid string copying sometimes) or a Twine 
> (in case you want to be fancy), but a string seems like it combines the worst 
> properties of all of these.
Actually, I think we should just use a StringRef and use llvm::errs() for 
printing.


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Commands/CommandObjectLog.cpp:167
   buffer_size.Clear();
+  handler = eLogHandlerStream;
   log_options = 0;

mib wrote:
> clayborg wrote:
> > Do we want to define a "eLogHandlerDefault" which points to 
> > "eLogHandlerStream"?
> I was hoping we could use a type alias for this, that marks 
> `eLogHandlerStream` as `eLogHandlerDefault` but I couldn't find of a nice C++ 
> way to do it :/ 
Were you maybe looking for something like:
```
enum {
 foo_1,
 foo_2,
 ...,
  foo_default = foo_47
};
```


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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1196
+  if (!bool(m_extensions_supported &
+NativeProcessProtocol::Extension::multiprocess))
+assert(!m_stdio_handle_up);

mgorny wrote:
> mgorny wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > labath wrote:
> > > > > I don't think this is right. I believe the important distinction is 
> > > > > whether we have non-stop mode enabled or not, because in all-stop 
> > > > > mode we are not able to send stdio while stopped, regardless of how 
> > > > > many processes we're debugging.
> > > > Well, the code exploded in all-stop mode as well because in order to 
> > > > kill multiple processes, we effectively resume them all. I suppose 
> > > > there could be a way around it (synchronously killing one process after 
> > > > another and waiting for everything to happen) but I'm not convinced 
> > > > this is really worth the added complexity.
> > > I don't think this needs to be complex at all. What we need to basically 
> > > do, is call StartSTDIOForwarding whenever the protocol allows us to send 
> > > stdio, and StopSTDIOForwarding when we cannot. When we supported just a 
> > > single process, the easiest way to achieve this was to hook into the 
> > > started/stopped events of that process. Now that's no longer true, so 
> > > maybe we just need to hook it up elsewhere.
> > > 
> > > I think starting could be done directly from the packet handlers 
> > > (c/s/vCont/...). And those handlers already have to check for nonstop 
> > > mode, so any deviations could be handled there:
> > > ```
> > > if (all_stop) {
> > >   StartSTDIOForwarding(); // Can forward from now until the stop-reply 
> > > packet is send
> > >   return Success;
> > > } else {
> > >   SendOKResponse();
> > >   // Can we forward now? Or maybe it can be sent all the time?
> > > }
> > > ```
> > > 
> > > Stopping could probably stay coupled with the sending of the stop-reply 
> > > packet (i.e., handling of the process event), since it's the stop reply 
> > > which determines whether the stdio packet can be sent.
> > Thanks, this makes sense. Good news is that it seems that I can just wire 
> > it into our current `SendContinueSuccessResponse()`, unless I've missed 
> > some other case (I've grepped for everything calling `Resume()` or 
> > `Kill()`). So far I didn't special-case non-stop mode, as I still need to 
> > rework `O` packets for it.
> > 
> > That said, do we want to enable forwarding for kill actions at all? I 
> > suppose this was kinda implicit due to the whole Linux `PTRACE_EVENT_EXIT` 
> > ping-pong but I honestly doubt any output can happen there.
> Oh, and regarding non-stop mode, I've left it as TODO for now. The whole 
> stdio forwarding needs to be fixed for non-stop mode anyway, and since we 
> don't expect to have two processes running simultaneously yet, I'm going to 
> look into it separately.
> 
> That said, this is probably going to be a "LLDB extension". FWICS gdb pretty 
> much uses `O` packets only to deliver debugger-specific messages and doesn't 
> forward stdio at all, nor special-cases `O` in non-stop mode. My initial idea 
> is to always send `O` as asynchronous notifications. I suppose we could then 
> enable stdio forwarding as soon as non-stop mode is enabled, and keep it 
> enabled until the very end.
Using async notifications sounds reasonable. Also, ew, I did not realize that's 
what O packets were meant for.

I don't think we need to enable forwarding for kill packets. In fact, I would 
be surprised if lldb was prepared to handle them here.


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

https://reviews.llvm.org/D127500

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added a comment.

Thanks for working on this 🙂
Looks good overall, just left some minor comments.




Comment at: lldb/include/lldb/Target/TraceCursor.h:64
 /// Low level traversal:
-///   Unlike the \a TraceCursor::Next() API, which uses a given granularity and
-///   direction to advance the cursor, the \a TraceCursor::Seek() method can be
+///   Unlike the \a TraceCursor::Next() API, which direction to advance
+//the cursor, the \a TraceCursor::Seek() method can be

For some reason I can't make inline suggestions, so I'll just put my suggested 
edit below:

s/which direction/which uses the current direction of the trace



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:105-107
+  void PrintInstructionType();
+
   void DumpInstructionDisassembly(const InstructionSymbolInfo &insn);

Don't forget to rebase onto mainline, @wallace's recent diff 
(https://reviews.llvm.org/D128316) refactored some of this code, so that will 
effect where some of these new changes should live.



Comment at: lldb/source/Commands/Options.td:1132
+  def thread_trace_dump_instructions_show_kind : Option<"kind", "k">, Group<1>,
+Desc<"For each instruction, print the instruction type">;
   def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">, Group<1>,

nit: add a period at the end of the description (:



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:46-50
 if (m_tsc_range && !m_tsc_range->InRange(m_pos))
   m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
 
-if (!m_ignore_errors && IsError())
-  return true;
-if (GetInstructionControlFlowType() & m_granularity)
-  return true;
+return true;
   }

Apologies again bc I can't "Suggest Edits" inline, so leaving my suggestion in 
a code block below:

IIUC, we previously had a while loop here because the possibility of "skipping" 
instructions due to ignoring errors or not matching the granularity. Since this 
diff removes those two things, this logic can now be simplified since the trace 
cursor can no longer skip the instructions - below is roughly what I'm thinking:
```
bool TraceCursorIntelPT::Next() {
  auto canMoveOne = [&]() {
if (IsForwards())
  return m_pos + 1 < GetInternalInstructionSize();
return m_pos > 0;
  };

  if (!canMoveOne())
return false;

  m_pos += IsForwards() ? 1 : -1;
  if (m_tsc_range && !m_tsc_range->InRange(m_pos))
m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();

  return true;
}
```
@wallace wdyt?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128477

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

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128366#3605484 , @bulbazord wrote:

> In D128366#3603943 , @labath wrote:
>
>> Could we move this pruning elsewhere? These values come from the symbol file 
>> plugins anyway, and they can do a better job at determining which language 
>> does a particular name belong to.
>> (OK, they can an also come from the symtab, but there I guess we could infer 
>> something from the mangling scheme).
>
> We're specifically pruning the results from a name lookup, so I'm not sure 
> where would be a better place to move it.

Well. I guess ideally I would pass whatever information is needed into the 
lookup functions themselves, so that there is no need for the additional 
filtering. I don't know why it's done this way but this setup seems 
inefficient, as we have to generate all these SymbolContext only for them to be 
(potentially) thrown away...




Comment at: lldb/include/lldb/Target/Language.h:320
+  virtual bool NamesAreEquivalentWithContext(
+  const ConstString &user_provided_name,
+  const ConstString &full_name_with_context) const {

bulbazord wrote:
> labath wrote:
> > I think we're passing ConstStrings by value these days...
> I see. Is there a reason we do that instead of passing by reference?
ConstString is just a fancy wrapper around a const char *, so adding a layer of 
indirection does not really save us anything (in fact, it's the opposite).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128366

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

[Lldb-commits] [PATCH] D127667: [lldb] [llgs] Implement the vKill packet

[Pavel Labath via Phabricator via lldb-commits]

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

Yeah, the pre-exit event makes this somewhat tricky. Let's stick with this then.


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

https://reviews.llvm.org/D127667

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

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128410#3604927 , @alvinhochun 
wrote:

> It may be possible to stuff a pointer to an `EXCEPTION_RECORD` into another 
> `EXCEPTION_RECORD` and use `RtlRaiseException` to generate the exception, but 
> you'll have to test how it actually works.

That would be pretty cool.




Comment at: lldb/test/Shell/Process/Windows/wndproc_exception.cpp:7
+// RUN: %clangxx_host -o %t.exe -luser32 -v -- %s
+// RUN: %lldb -f %t.exe -o "run"
+

alvinhochun wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > mstorsjo wrote:
> > > > labath wrote:
> > > > > Is there something reasonable we could assert here? The process exit 
> > > > > status for instance?
> > > > This in itself requires the `%lldb` command to succeed - the exit 
> > > > status for each `RUN` line needs to be successful (unless a failure is 
> > > > expected and it can be inverted with the `not` command). Or did you 
> > > > mean checking the process exit code of the child process (that 
> > > > intentionally does crash)?
> > > > 
> > > > When this test case is run, it prints the following to the terminal:
> > > > ```
> > > > (lldb) run
> > > > Process 3168 stopped
> > > > * thread #1, stop reason = Exception 0xc41d encountered at address 
> > > > 0x7ff68e6f1227
> > > > frame #0: 0x7ff68e6f1227 wndproc_exception.cpp.tmp.exe
> > > > ->  0x7ff68e6f1227: movl   $0x1, (%rax)
> > > > 0x7ff68e6f122d: movq   $0x0, 0x50(%rsp)
> > > > 0x7ff68e6f1236: jmp0x7ff68e6f1259
> > > > 0x7ff68e6f123b: movq   0x48(%rsp), %r9
> > > > Process 3168 launched: 
> > > > 'C:\dev\llvm-project\llvm\build-msvc\tools\lldb\test\Shell\Process\Windows\Output\wndproc_exception.cpp.tmp.exe'
> > > >  (x86_64)
> > > > ```
> > > > I guess we could check for `Exception 0xc41d encountered` maybe, 
> > > > although I'm afraid of making the testcase unnecessarily brittle too.
> > > If running with lldb-server enabled, it prints a different exception 
> > > code, `0xc005` (which is `STATUS_ACCESS_VIOLATION`) which probably is 
> > > the proper nested exception, while without lldb-server, it prints 
> > > `0xc41d` (`STATUS_FATAL_USER_CALLBACK_EXCEPTION`).
> > > 
> > > So for the purpose of this testcase, just to make sure that it doesn't 
> > > crash in this case, it'd be better to not specify exactly which exception 
> > > code is to be returned.
> > It may be that lldb-server is catching the first chance 
> > `STATUS_ACCESS_VIOLATION` exception before the exception is passed to the 
> > exception handler...
> If you test with windbg or gdb, you will get the access violation / sigsegv 
> first, and only after continuing the debuggee you will get the `0xc41d` 
> exception.
> This in itself requires the `%lldb` command to succeed - the exit status for 
> each `RUN` line needs to be successful (unless a failure is expected and it 
> can be inverted with the `not` command). Or did you mean checking the process 
> exit code of the child process (that intentionally does crash)?

Yes, that's what I mean, but I don't think we need to match the precise 
exception number (although probably only one of those two numbers is really 
"correct"). We could just match the `stop reason = Exception` part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128410

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128268#3604555 , @mstorsjo wrote:

> In D128268#3604081 , @labath wrote:
>
>>> If we'd just set this to the baseline, i386, would that have any effect for 
>>> how lldb e.g. is able to disassemble/interpret instructions that don't 
>>> exist in the i386 baseline architecture?
>>
>> It should not have any effect (if it does, that's a separate fix). In the 
>> disassembler, we explicitly enable the latest instruction set, and I can't 
>> think of anything else that would be impacted by it.
>
> Thanks - I did some cursory testing with removing the extra i686 everywhere, 
> and at least on a quick test, it seems to work just fine (and requires a 
> minor adjustment to only one testcase).
>
> I found that this duality was introduced in 
> 5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 
>  and ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 
> / D4658  (CC @zturner), what do you make out 
> of the reasonings in those commits?

The first patch seems like it's just working around some mismatches in the 
windows dynamic loader plugin. I think a better approach would be to have the 
dynamic loader claim both architectures, though I don't think that is necessary 
if we're just consistent about what we use. I don't see anything wrong with the 
second patch (the darwin platform does something similar for arm architectures, 
even though I'm not convinced it's necessary (the reason it's necessary for 
darwin is because there they actually make a distinction between armv6XX and 
armv7YY and treat those as different architectures).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This isn't turning out as clean as I had hoped it would, but I think it's a 
step in the right direction. Jonas, do you want to say anything about this?




Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:54
+  const DWARFExpression *
+  GetExpressionAtAddress(lldb::addr_t file_addr = 0) const;
+

calling GetExpressionAtAddress without an address seems weird. Maybe just have 
a separate GetAlwaysValidExpr getter or something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

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

[Lldb-commits] [PATCH] D128484: [NFC][lldb][trace] Rename trace session to trace bundle

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Core/PluginManager.h:346
 
-  static TraceCreateInstanceForSessionFile
+  static TraceCreateInstanceForBundle
   GetTraceCreateCallback(llvm::StringRef plugin_name);

jj10306 wrote:
> nit:  `FromBundle` makes more sense than `ForBundle` imo
makes sense! will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128484

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

[Lldb-commits] [lldb] e8fe7e9 - [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: e8fe7e930a45764cbb88d9c3fa91ef7dc1ebcc97

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

LOG: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

Modify the behavior of the `k` packet to kill all inferiors rather than
just the current one.  The specification leaves the exact behavior
of this packet up to the implementation but since vKill is specifically
meant to be used to kill a single process, it seems logical to use `k`
to provide the alternate function of killing all of them.

Move starting stdio forwarding from the "running" response
to the packet handlers that trigger the process to start.  This avoids
attempting to start it multiple times when multiple processes are killed
on Linux which implicitly causes LLGS to receive "started" events
for all of them.  This is probably also more correct as the ability
to send "O" packets is implied by the continue-like command being issued
(and therefore the client waiting for responses) rather than the start
notification.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127500

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1cddb5b29c595..434fb50862f8b 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1041,17 +1041,26 @@ void 
GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited(
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
-
-  // When running in non-stop mode, wait for the vStopped to clear
-  // the notification queue.
-  if (!m_non_stop) {
-// We are ready to exit the debug monitor.
-m_exit_now = true;
-m_mainloop.RequestTermination();
-  }
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m_continue_process = nullptr;
+
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([this, pid](MainLoopBase &loop) {
+m_debugged_processes.erase(pid);
+// When running in non-stop mode, wait for the vStopped to clear
+// the notification queue.
+if (m_debugged_processes.empty() && !m_non_stop) {
+  // Close the pipe to the inferior terminal i/o if we launched it and set
+  // one up.
+  MaybeCloseInferiorTerminalConnection();
+
+  // We are ready to exit the debug monitor.
+  m_exit_now = true;
+  loop.RequestTermination();
+}
+  });
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1094,7 +1103,6 @@ void 
GDBRemoteCommunicationServerLLGS::ProcessStateChanged(
 
   switch (state) {
   case StateType::eStateRunning:
-StartSTDIOForwarding();
 break;
 
   case StateType::eStateStopped:
@@ -1219,7 +1227,7 @@ void 
GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
 return;
 
   Status error;
-  lldbassert(!m_stdio_handle_up);
+  assert(!m_stdio_handle_up);
   m_stdio_handle_up = m_mainloop.RegisterReadObject(
   m_stdio_communication.GetConnection()->GetReadObject(),
   [this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1228,10 +1236,7 @@ void 
GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
 // Not much we can do about the failure. Log it and continue without
 // forwarding.
 if (Log *log = GetLog(LLDBLog::Process))
-  LLDB_LOGF(log,
-"GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio "
-"forwarding: %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOG(log, "Failed to set up stdio forwarding: {0}", error);
   }
 }
 
@@ -1416,15 +1421,19 @@ 
GDBRemoteCommunicationServerLLGS::Handle_k(StringExtractorGDBRemote &packet) {
 
   StopSTDIOForwarding();
 
-  if (!m_current_process) {
+  if (m_debugged_processes.empty()) {
 LLDB_LOG(log, "No debugged process found.");
 return PacketResult::Success;
   }
 
-  Status error = m_current_process->Kill();
-  if (error.Fail())
-LLDB_LOG(log, "Failed to kill debugged process {0}: {1}",
- m_current_process->GetID(), error);
+  for (auto it = m_debugged_processes.begin(); it != 
m_debugged_processes.end();
+   ++it) {
+LLDB_LOG(log, "Killing process {0}", it->first);
+Status error = it->second->Kill();
+  

[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8fe7e930a45: [lldb] [llgs] Make `k` kill all processes, and 
fix multiple exits (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127500

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -262,3 +262,37 @@
 "send packet: $Eff#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_kill_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+child_pid = ret["child_pid"]
+self.reset_test_sequence()
+
+exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+self.test_sequence.add_log_lines([
+# kill all processes
+"read packet: $k#00",
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertEqual(set([ret["pid1"], ret["pid2"]]),
+ set([parent_pid, child_pid]))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1041,17 +1041,26 @@
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
-
-  // When running in non-stop mode, wait for the vStopped to clear
-  // the notification queue.
-  if (!m_non_stop) {
-// We are ready to exit the debug monitor.
-m_exit_now = true;
-m_mainloop.RequestTermination();
-  }
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m_continue_process = nullptr;
+
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([this, pid](MainLoopBase &loop) {
+m_debugged_processes.erase(pid);
+// When running in non-stop mode, wait for the vStopped to clear
+// the notification queue.
+if (m_debugged_processes.empty() && !m_non_stop) {
+  // Close the pipe to the inferior terminal i/o if we launched it and set
+  // one up.
+  MaybeCloseInferiorTerminalConnection();
+
+  // We are ready to exit the debug monitor.
+  m_exit_now = true;
+  loop.RequestTermination();
+}
+  });
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1094,7 +1103,6 @@
 
   switch (state) {
   case StateType::eStateRunning:
-StartSTDIOForwarding();
 break;
 
   case StateType::eStateStopped:
@@ -1219,7 +1227,7 @@
 return;
 
   Status error;
-  lldbassert(!m_stdio_handle_up);
+  assert(!m_stdio_handle_up);
   m_stdio_handle_up = m_mainloop.RegisterReadObject(
   m_stdio_communication.GetConnection()->GetReadObject(),
   [this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1228,10 +1236,7 @@
 // Not much we can do about the failure. Log it and continue without
 // forwarding.
 if (Log *log = GetLog(LLDBLog::Process))
-  LLDB_LOGF(log,
-"GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio "
-"forwarding: %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOG(log, "Failed to set up stdio forwarding: {0}", error);
   }
 }
 
@@ -1416,15 +1421,19 @@
 
   StopSTDIOForwarding();
 
-  if (!m_current_process) {
+  if (m_debugged_processes.empty()) {
 LLDB_LOG(log, "No debugged process found.");
 return PacketResult::Success;
   }
 
-  Status error = m_current_process->Kill();
-  if (error.Fail())
-LLDB_LOG(log, "Failed to k

[Lldb-commits] [lldb] c18784b - [lldb] [llgs] Implement the vKill packet

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: c18784ba330ac0f57e6ec433cfa8317349c445ff

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

LOG: [lldb] [llgs] Implement the vKill packet

Implement the support for the vKill packet.  This is the modern packet
used by the GDB Remote Serial Protocol to kill one of the debugged
processes.  Unlike the `k` packet, it has well-defined semantics.

The `vKill` packet takes the PID of the process to kill, and always
replies with an `OK` reply (rather than the exit status, as LLGS does
for `k` packets at the moment).  Additionally, unlike the `k` packet
it does not cause the connection to be terminated once the last process
is killed — the client needs to close it explicitly.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127667

Added: 


Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 2a63212e9d287..c32ce0389116e 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -136,6 +136,7 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_vAttachName,
 eServerPacketType_vCont,
 eServerPacketType_vCont_actions, // vCont?
+eServerPacketType_vKill,
 eServerPacketType_vRun,
 
 eServerPacketType_stop_reason, // '?'

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 434fb50862f8b..1ff0b7fe4ee5d 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -232,6 +232,10 @@ void 
GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
   return this->Handle_k(packet);
 });
 
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_vKill,
+  &GDBRemoteCommunicationServerLLGS::Handle_vKill);
+
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
   &GDBRemoteCommunicationServerLLGS::Handle_qSaveCore);
@@ -482,6 +486,10 @@ GDBRemoteCommunicationServerLLGS::SendWResponse(
   LLDB_LOG(log, "pid = {0}, returning exit type {1}", process->GetID(),
*wait_status);
 
+  // If the process was killed through vKill, return "OK".
+  if (m_vkilled_processes.find(process->GetID()) != m_vkilled_processes.end())
+return SendOKResponse();
+
   StreamGDBRemote response;
   response.Format("{0:g}", *wait_status);
   if (bool(m_extensions_supported & 
NativeProcessProtocol::Extension::multiprocess))
@@ -1049,9 +1057,13 @@ void 
GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited(
   lldb::pid_t pid = process->GetID();
   m_mainloop.AddPendingCallback([this, pid](MainLoopBase &loop) {
 m_debugged_processes.erase(pid);
+auto vkill_it = m_vkilled_processes.find(pid);
+if (vkill_it != m_vkilled_processes.end())
+  m_vkilled_processes.erase(vkill_it);
+// Terminate the main loop only if vKill has not been used.
 // When running in non-stop mode, wait for the vStopped to clear
 // the notification queue.
-if (m_debugged_processes.empty() && !m_non_stop) {
+else if (m_debugged_processes.empty() && !m_non_stop) {
   // Close the pipe to the inferior terminal i/o if we launched it and set
   // one up.
   MaybeCloseInferiorTerminalConnection();
@@ -1442,6 +1454,30 @@ 
GDBRemoteCommunicationServerLLGS::Handle_k(StringExtractorGDBRemote &packet) {
   return SendContinueSuccessResponse();
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vKill(
+StringExtractorGDBRemote &packet) {
+  StopSTDIOForwarding();
+
+  packet.SetFilePos(6); // vKill;
+  uint32_t pid = packet.GetU32(LLDB_INVALID_PROCESS_ID, 16);
+  if (pid == LLDB_INVALID_PROCESS_ID)
+return SendIllFormedResponse(packet,
+ "vKill failed to parse the process id");
+
+  auto it = m_debugged_processes.find(pid);
+  if (it == m_debugged_processes.end())
+return SendErrorResponse(42);
+
+  Status error = it->second->Kill();
+  if (error.Fail())
+return SendErrorResponse(error.ToError());
+
+  // OK response is s

[Lldb-commits] [lldb] 3266b11 - [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: 3266b117147db73d1c42668c1033b59a36d8a2f3

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

LOG: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

Add a test verifying that it is possible to resume a single process
via the `c` packet when multiple processes are being debugged.  This
includes a tiny change to the test program — when `fork()` is called,
the child process is no longer terminated immediately but continues
performing the same tasks as queued for the parent.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127755

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
lldb/test/API/tools/lldb-server/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index a49a5b42c2ba8..f5f088f506848 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -353,3 +353,81 @@ def test_vkill_parent(self):
 @add_test_categories(["fork"])
 def test_vkill_both(self):
 self.vkill_test(kill_parent=True, kill_child=True)
+
+def resume_one_test(self, run_order):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+parent_expect = [
+"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"[$]W00;process:{}#.*".format(parent_pid),
+]
+child_expect = [
+"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
+"[$]W00;process:{}#.*".format(child_pid),
+]
+
+for x in run_order:
+if x == "parent":
+pidtid = (parent_pid, parent_tid)
+expect = parent_expect.pop(0)
+elif x == "child":
+pidtid = (child_pid, child_tid)
+expect = child_expect.pop(0)
+else:
+assert False, "unexpected x={}".format(x)
+
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $Hcp{}.{}#00".format(*pidtid),
+"send packet: $OK#00",
+"read packet: $c#00",
+{"direction": "send", "regex": expect},
+], True)
+# if at least one process remained, check both PIDs
+if parent_expect or child_expect:
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: ${}#00".format("OK" if parent_expect else 
"Eff"),
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: ${}#00".format("OK" if child_expect else 
"Eff"),
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_c_parent(self):
+self.resume_one_test(run_order=["parent", "parent"])
+
+@add_test_categories(["fork"])
+def test_c_child(self):
+self.resume_one_test(run_order=["child", "child"])
+
+@add_test_categories(["fork"])
+def test_c_parent_then_child(self):
+self.resume_one_test(run_order=["parent", "parent", "child", "child"])
+
+@add_test_categories(["fork"])
+def test_c_child_then_parent(self):
+self.resume_one_test(run_order=["child", "child", "parent", "parent"])
+
+@add_test_categories(["fork"])
+def test_c_interspersed(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"])

diff  --git a/lldb/test/API/tools/lldb-server/main.cpp 
b/lldb/test/API/tools/lldb-server/main.cpp
index 0383acc68fccf..9907a09dfc7d2 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32

[Lldb-commits] [PATCH] D127667: [lldb] [llgs] Implement the vKill packet

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc18784ba330a: [lldb] [llgs] Implement the vKill packet 
(authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D127667?vs=438819&id=439769#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127667

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -296,3 +296,60 @@
 ret = self.expect_gdbremote_sequence()
 self.assertEqual(set([ret["pid1"], ret["pid2"]]),
  set([parent_pid, child_pid]))
+
+def vkill_test(self, kill_parent=False, kill_child=False):
+assert kill_parent or kill_child
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+if kill_parent:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(parent_pid),
+"send packet: $OK#00",
+], True)
+if kill_child:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(child_pid),
+"send packet: $OK#00",
+], True)
+self.test_sequence.add_log_lines([
+# check child PID/TID
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: ${}#00".format("Eff" if kill_child else "OK"),
+# check parent PID/TID
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: ${}#00".format("Eff" if kill_parent else "OK"),
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_vkill_child(self):
+self.vkill_test(kill_child=True)
+
+@add_test_categories(["fork"])
+def test_vkill_parent(self):
+self.vkill_test(kill_parent=True)
+
+@add_test_categories(["fork"])
+def test_vkill_both(self):
+self.vkill_test(kill_parent=True, kill_child=True)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -372,6 +372,8 @@
 return eServerPacketType_vCont;
   if (PACKET_MATCHES("vCont?"))
 return eServerPacketType_vCont_actions;
+  if (PACKET_STARTS_WITH("vKill;"))
+return eServerPacketType_vKill;
   if (PACKET_STARTS_WITH("vRun;"))
 return eServerPacketType_vRun;
   if (PACKET_MATCHES("vStopped"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/MainLoop.h"
@@ -95,6 +96,7 @@
   std::recursive_mutex m_debugged_process_mutex;
   std::unordered_map>
   m_debugged_processes;
+  std::unordered_set m_vkilled_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
@@ -129,6 +131,8 @@
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vKill(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_qProcessInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qC(Stri

[Lldb-commits] [lldb] a342279 - [lldb] [llgs] Support resuming one process with PID!=current via vCont

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: a3422793e0643fa849ff178d87fc706c81b734b7

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

LOG: [lldb] [llgs] Support resuming one process with PID!=current via vCont

Extend vCont function to support resuming a process with an arbitrary
PID, that could be different than the one selected via Hc (or no process
at all may be selected).  Resuming more than one process simultaneously
is not supported yet.

Remove the ReadTid() method that was only used by Handle_vCont(),
and furthermore it was wrongly using m_current_process rather than
m_continue_process.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127862

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1ff0b7fe4ee5d..f6b61f521206b 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1674,12 +1674,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
 return SendIllFormedResponse(packet, "Missing action from vCont package");
   }
 
-  // Check if this is all continue (no options or ";c").
-  if (::strcmp(packet.Peek(), ";c") == 0) {
-// Move past the ';', then do a simple 'c'.
-packet.SetFilePos(packet.GetFilePos() + 1);
-return Handle_c(packet);
-  } else if (::strcmp(packet.Peek(), ";s") == 0) {
+  if (::strcmp(packet.Peek(), ";s") == 0) {
 // Move past the ';', then do a simple 's'.
 packet.SetFilePos(packet.GetFilePos() + 1);
 return Handle_s(packet);
@@ -1688,13 +1683,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
 return SendOKResponse();
   }
 
-  // Ensure we have a native process.
-  if (!m_continue_process) {
-LLDB_LOG(log, "no debugged process");
-return SendErrorResponse(0x36);
-  }
-
-  ResumeActionList thread_actions;
+  std::unordered_map thread_actions;
 
   while (packet.GetBytesLeft() && *packet.Peek() == ';') {
 // Skip the semi-colon.
@@ -1737,32 +1726,62 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   break;
 }
 
+lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
+lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
+
 // Parse out optional :{thread-id} value.
 if (packet.GetBytesLeft() && (*packet.Peek() == ':')) {
   // Consume the separator.
   packet.GetChar();
 
-  llvm::Expected tid_ret =
-  ReadTid(packet, /*allow_all=*/true, m_continue_process->GetID());
-  if (!tid_ret)
-return SendErrorResponse(tid_ret.takeError());
+  auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+  if (!pid_tid)
+return SendIllFormedResponse(packet, "Malformed thread-id");
 
-  thread_action.tid = tid_ret.get();
-  if (thread_action.tid == StringExtractorGDBRemote::AllThreads)
-thread_action.tid = LLDB_INVALID_THREAD_ID;
+  pid = pid_tid->first;
+  tid = pid_tid->second;
 }
 
-thread_actions.Append(thread_action);
-  }
+if (pid == StringExtractorGDBRemote::AllProcesses) {
+  if (m_debugged_processes.size() > 1)
+return SendIllFormedResponse(
+packet, "Resuming multiple processes not supported yet");
+  if (!m_continue_process) {
+LLDB_LOG(log, "no debugged process");
+return SendErrorResponse(0x36);
+  }
+  pid = m_continue_process->GetID();
+}
 
-  Status error = m_continue_process->Resume(thread_actions);
-  if (error.Fail()) {
-LLDB_LOG(log, "vCont failed for process {0}: {1}",
- m_continue_process->GetID(), error);
-return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+if (tid == StringExtractorGDBRemote::AllThreads)
+  tid = LLDB_INVALID_THREAD_ID;
+
+thread_action.tid = tid;
+
+thread_actions[pid].Append(thread_action);
   }
 
-  LLDB_LOG(log, "continued process {0}", m_continue_process->GetID());
+  assert(thread_actions.size() >= 1);
+  if (thread_actions.size() > 1)
+return SendIllFormedResponse(
+packet, "Resuming multiple processes not supported yet");
+
+  for (std::pair x : thread_actions) {
+auto process_it = m_debugged_processes.find(x.first);
+if (process_it == m_debugged_processes.end()) {
+  LLDB_LOG(log, "vCont failed for process {0}: process n

[Lldb-commits] [lldb] 0481d8e - [lldb] [llgs] Add a test for multiprocess memory read/write

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: 0481d8efa92f4508e847046b748c17ace1813272

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

LOG: [lldb] [llgs] Add a test for multiprocess memory read/write

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128150

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index ec6ae2575f4e3..31dd20b475c58 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -553,3 +553,90 @@ def test_vCont_all_processes_implicit(self):
 "send packet: $E03#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_memory_read_write(self):
+self.build()
+INITIAL_DATA = "Initial message"
+self.prep_debug_monitor_and_inferior(
+inferior_args=["set-message:{}".format(INITIAL_DATA),
+   "get-data-address-hex:g_message",
+   "fork",
+   "print-message:",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"type": "output_match",
+ "regex": self.maybe_strict_output_regex(r"data address: 
0x([0-9a-fA-F]+)\r\n"),
+ "capture": {1: "addr"}},
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+pidtids = {
+"parent": (ret["parent_pid"], ret["parent_tid"]),
+"child": (ret["child_pid"], ret["child_tid"]),
+}
+addr = ret["addr"]
+self.reset_test_sequence()
+
+for name, pidtid in pidtids.items():
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # read the current memory contents
+ "read packet: $m{},{:x}#00".format(addr,
+len(INITIAL_DATA) + 1),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ # write a new value
+ "read packet: $M{},{:x}:{}#00".format(addr,
+   len(name) + 1,
+   seven.hexlify(
+   name + "\0")),
+ "send packet: $OK#00",
+ # resume the process and wait for the trap
+ "read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"type": "output_match",
+  "regex": self.maybe_strict_output_regex(r"message: 
(.*)\r\n"),
+  "capture": {1: "printed_message"}},
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+ret = self.expect_gdbremote_sequence()
+data = seven.unhexlify(ret["data"])
+self.assertEqual(data, INITIAL_DATA + "\0")
+self.assertEqual(ret["printed_message"], name);
+self.reset_test_sequence()
+
+# we do the second round separately to make sure that initial data
+# is correctly preserved while writing into the first process
+
+for name, pidtid in pidtids.items():
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # read the current memory contents
+ "read packet: $m{},{:x}#00".format(addr,
+len(name) + 1),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ ], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIsNotNone(ret.get("data"))
+data = seve

[Lldb-commits] [lldb] 75757c8 - [lldb] [llgs] Support multiprocess in qfThreadInfo

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: 75757c86c695a6b4695458343637b3c4fe86def6

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

LOG: [lldb] [llgs] Support multiprocess in qfThreadInfo

Update the `qfThreadInfo` handler to report threads of all debugged
processes and include PIDs when in multiprocess mode.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128152

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
index d1d265d8852b5..eff6dd6d53007 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -284,9 +284,14 @@ def parse_threadinfo_response(response_packet):
 response_packet = _STRIP_COMMAND_PREFIX_M_REGEX.sub("", response_packet)
 response_packet = _STRIP_CHECKSUM_REGEX.sub("", response_packet)
 
-# Return list of thread ids
-return [int(thread_id_hex, 16) for thread_id_hex in response_packet.split(
-",") if len(thread_id_hex) > 0]
+for tid in response_packet.split(","):
+if not tid:
+continue
+if tid.startswith("p"):
+pid, _, tid = tid.partition(".")
+yield (int(pid[1:], 16), int(tid, 16))
+else:
+yield int(tid, 16)
 
 
 def unpack_endian_binary_string(endian, value_string):

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index f6b61f521206b..8c26032cc90db 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1974,38 +1974,46 @@ GDBRemoteCommunicationServerLLGS::Handle_qRegisterInfo(
   return SendPacketNoLock(response.GetString());
 }
 
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
-StringExtractorGDBRemote &packet) {
+void GDBRemoteCommunicationServerLLGS::AddProcessThreads(
+StreamGDBRemote &response, NativeProcessProtocol &process, bool &had_any) {
   Log *log = GetLog(LLDBLog::Thread);
 
-  // Fail if we don't have a current process.
-  if (!m_current_process ||
-  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-LLDB_LOG(log, "no process ({0}), returning OK",
- m_current_process ? "invalid process id"
-   : "null m_current_process");
-return SendOKResponse();
-  }
-
-  StreamGDBRemote response;
-  response.PutChar('m');
+  lldb::pid_t pid = process.GetID();
+  if (pid == LLDB_INVALID_PROCESS_ID)
+return;
 
-  LLDB_LOG(log, "starting thread iteration");
+  LLDB_LOG(log, "iterating over threads of process {0}", process.GetID());
   NativeThreadProtocol *thread;
   uint32_t thread_index;
-  for (thread_index = 0,
-  thread = m_current_process->GetThreadAtIndex(thread_index);
-   thread; ++thread_index,
-  thread = m_current_process->GetThreadAtIndex(thread_index)) {
-LLDB_LOG(log, "iterated thread {0}(tid={2})", thread_index,
+  for (thread_index = 0, thread = process.GetThreadAtIndex(thread_index);
+   thread;
+   ++thread_index, thread = process.GetThreadAtIndex(thread_index)) {
+LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
  thread->GetID());
-if (thread_index > 0)
-  response.PutChar(',');
-response.Printf("%" PRIx64, thread->GetID());
+response.PutChar(had_any ? ',' : 'm');
+if (bool(m_extensions_supported &
+ NativeProcessProtocol::Extension::multiprocess))
+  response.Format("p{0:x-}.", pid);
+response.Format("{0:x-}", thread->GetID());
+had_any = true;
   }
+}
 
-  LLDB_LOG(log, "finished thread iteration");
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
+StringExtractorGDBRemote &packet) {
+  assert(m_debugged_processes.size() == 1 ||
+ bool(m_extensions_supported &
+  NativeProcessProtocol::Extension::multiprocess));
+
+  bool had_any = false;
+  StreamGDBRemote response;
+
+  for (auto &pid_ptr : m_debugged_processes)
+AddProcessThreads(response, *pid_ptr.second, had_any);
+
+  if (!had_any)
+respo

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3266b117147d: [lldb] [llgs] Add test for resuming via c in 
multiprocess scenarios (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -323,8 +324,7 @@
   func_p();
 #if !defined(_WIN32) && !defined(TARGET_OS_WATCH) && !defined(TARGET_OS_TV)
 } else if (arg == "fork") {
-  if (fork() == 0)
-_exit(0);
+  assert (fork() != -1);
 } else if (arg == "vfork") {
   if (vfork() == 0)
 _exit(0);
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -353,3 +353,81 @@
 @add_test_categories(["fork"])
 def test_vkill_both(self):
 self.vkill_test(kill_parent=True, kill_child=True)
+
+def resume_one_test(self, run_order):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+parent_expect = [
+"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"[$]W00;process:{}#.*".format(parent_pid),
+]
+child_expect = [
+"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
+"[$]W00;process:{}#.*".format(child_pid),
+]
+
+for x in run_order:
+if x == "parent":
+pidtid = (parent_pid, parent_tid)
+expect = parent_expect.pop(0)
+elif x == "child":
+pidtid = (child_pid, child_tid)
+expect = child_expect.pop(0)
+else:
+assert False, "unexpected x={}".format(x)
+
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $Hcp{}.{}#00".format(*pidtid),
+"send packet: $OK#00",
+"read packet: $c#00",
+{"direction": "send", "regex": expect},
+], True)
+# if at least one process remained, check both PIDs
+if parent_expect or child_expect:
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: ${}#00".format("OK" if parent_expect else "Eff"),
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: ${}#00".format("OK" if child_expect else "Eff"),
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_c_parent(self):
+self.resume_one_test(run_order=["parent", "parent"])
+
+@add_test_categories(["fork"])
+def test_c_child(self):
+self.resume_one_test(run_order=["child", "child"])
+
+@add_test_categories(["fork"])
+def test_c_parent_then_child(self):
+self.resume_one_test(run_order=["parent", "parent", "child", "child"])
+
+@add_test_categories(["fork"])
+def test_c_child_then_parent(self):
+self.resume_one_test(run_order=["child", "child", "parent", "parent"])
+
+@add_test_categories(["fork"])
+def test_c_interspersed(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 14d6707 - [lldb] [llgs] Add a test for multiprocess register read/write

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: 14d67073359a86f1d2ae9e140f0b29aa4e63a3af

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

LOG: [lldb] [llgs] Add a test for multiprocess register read/write

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128153

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 31f8a1336b58f..78e7d24f0db93 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -1,3 +1,5 @@
+import random
+
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -700,3 +702,118 @@ def test_memory_read_write(self):
 data = seven.unhexlify(ret.get("data"))
 self.assertEqual(data, name + "\0")
 self.reset_test_sequence()
+
+@add_test_categories(["fork"])
+def test_register_read_write(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(len(pidtids), 4)
+# first, save register values from all the threads
+thread_regs = {}
+for pidtid in pidtids:
+for regno in range(256):
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $p{:x}#00".format(regno),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ ], True)
+ret = self.expect_gdbremote_sequence()
+data = ret.get("data")
+self.assertIsNotNone(data)
+# ignore registers shorter than 32 bits (this also catches
+# "Exx" errors)
+if len(data) >= 8:
+break
+else:
+self.skipTest("no usable register found")
+thread_regs[pidtid] = (regno, data)
+
+vals = set(x[1] for x in thread_regs.values())
+# NB: cheap hack to make the loop below easier
+new_val = next(iter(vals))
+
+# then, start altering them and verify that we don't unexpectedly
+# change the value from another thread
+for pidtid in pidtids:
+old_val = thread_regs[pidtid]
+regno = old_val[0]
+old_val_length = len(old_val[1])
+# generate a unique new_val
+while new_val in vals:
+new_val = ('{{:0{}x}}'.format(old_val_length)
+   .format(random.getrandbits(old_val_length*4)))
+vals.add(new_val)
+
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $p{:x}#00".format(regno),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ 

[Lldb-commits] [lldb] 630da0e - [lldb] [llgs] Include PID in QC response in multiprocess mode

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: 630da0e309ef4764465dcaf559676633e948f5c0

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

LOG: [lldb] [llgs] Include PID in QC response in multiprocess mode

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128156

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 8c26032cc90db..18712cf5b323b 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1422,7 +1422,10 @@ 
GDBRemoteCommunicationServerLLGS::Handle_qC(StringExtractorGDBRemote &packet) {
 return SendErrorResponse(69);
 
   StreamString response;
-  response.Printf("QC%" PRIx64, thread->GetID());
+  response.PutCString("QC");
+  if (bool(m_extensions_supported & 
NativeProcessProtocol::Extension::multiprocess))
+response.Format("p{0:x-}.", m_current_process->GetID());
+  response.Format("{0:x-}", thread->GetID());
 
   return SendPacketNoLock(response.GetString());
 }

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 78e7d24f0db93..fdd9e94d0a364 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -11,7 +11,6 @@ class 
TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
-procinfo_regex = "[$]pid:([0-9a-f]+);.*"
 
 @add_test_categories(["fork"])
 def test_fork_multithreaded(self):
@@ -68,7 +67,7 @@ def fork_and_detach_test(self, variant):
 "send packet: $OK#00",
 # verify that the current process is correct
 "read packet: $qC#00",
-"send packet: $QC{}#00".format(parent_tid),
+"send packet: $QCp{}.{}#00".format(parent_pid, parent_tid),
 # verify that the correct processes are detached/available
 "read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
 "send packet: $Eff#00",
@@ -167,12 +166,9 @@ def test_select_wrong_pid(self):
 
 # get process pid
 self.test_sequence.add_log_lines([
-"read packet: $qProcessInfo#00",
-{"direction": "send", "regex": self.procinfo_regex,
- "capture": {1: "pid"}},
 "read packet: $qC#00",
-{"direction": "send", "regex": "[$]QC([0-9a-f]+)#.*",
- "capture": {1: "tid"}},
+{"direction": "send", "regex": "[$]QCp([0-9a-f]+).([0-9a-f]+)#.*",
+ "capture": {1: "pid", 2: "tid"}},
 ], True)
 ret = self.expect_gdbremote_sequence()
 pid, tid = (int(ret[x], 16) for x in ("pid", "tid"))
@@ -208,8 +204,8 @@ def test_detach_current(self):
 
 # get process pid
 self.test_sequence.add_log_lines([
-"read packet: $qProcessInfo#00",
-{"direction": "send", "regex": self.procinfo_regex,
+"read packet: $qC#00",
+{"direction": "send", "regex": "[$]QCp([0-9a-f]+).[0-9a-f]+#.*",
  "capture": {1: "pid"}},
 ], True)
 ret = self.expect_gdbremote_sequence()
@@ -817,3 +813,56 @@ def test_register_read_write(self):
 data = ret.get("data")
 self.assertIsNotNone(data)
 self.assertEqual(data, old_val[1])
+
+@add_test_categories(["fork"])
+def test_qC(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_

[Lldb-commits] [PATCH] D127862: [lldb] [llgs] Support resuming one process with PID!=current via vCont

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3422793e064: [lldb] [llgs] Support resuming one process 
with PID!=current via vCont (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127862

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -354,7 +354,7 @@
 def test_vkill_both(self):
 self.vkill_test(kill_parent=True, kill_child=True)
 
-def resume_one_test(self, run_order):
+def resume_one_test(self, run_order, use_vCont=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
 self.add_qSupported_packets(["multiprocess+",
@@ -395,11 +395,19 @@
 else:
 assert False, "unexpected x={}".format(x)
 
+if use_vCont:
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $vCont;c:p{}.{}#00".format(*pidtid),
+], True)
+else:
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $Hcp{}.{}#00".format(*pidtid),
+"send packet: $OK#00",
+"read packet: $c#00",
+], True)
 self.test_sequence.add_log_lines([
-# continue the selected process
-"read packet: $Hcp{}.{}#00".format(*pidtid),
-"send packet: $OK#00",
-"read packet: $c#00",
 {"direction": "send", "regex": expect},
 ], True)
 # if at least one process remained, check both PIDs
@@ -431,3 +439,117 @@
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
+
+@add_test_categories(["fork"])
+def test_vCont_parent(self):
+self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_child(self):
+self.resume_one_test(run_order=["child", "child"], use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_parent_then_child(self):
+self.resume_one_test(run_order=["parent", "parent", "child", "child"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_child_then_parent(self):
+self.resume_one_test(run_order=["child", "child", "parent", "parent"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_interspersed(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_two_processes(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+# try to resume both processes
+"read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+parent_pid, parent_tid, child_pid, child_tid),
+"send packet: $E03#00",
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_vCont_all_processes_explicit(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+",

[Lldb-commits] [lldb] e827e51 - [lldb] [llgs] Implement the 'T' packet

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: e827e5186fb6991bc749eaaddf13f8a53ebb63c2

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

LOG: [lldb] [llgs] Implement the 'T' packet

Implement the 'T' packet that is used to verify whether the specified
thread belongs to the debugged processes.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128170

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 18712cf5b323b..f5c66496d7659 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -107,6 +107,8 @@ void 
GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
 &GDBRemoteCommunicationServerLLGS::Handle_P);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qC,
 &GDBRemoteCommunicationServerLLGS::Handle_qC);
+  RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_T,
+&GDBRemoteCommunicationServerLLGS::Handle_T);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_qfThreadInfo,
   &GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo);
@@ -3898,6 +3900,36 @@ GDBRemoteCommunicationServerLLGS::Handle_vCtrlC(
   return SendOKResponse();
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_T(StringExtractorGDBRemote &packet) {
+  packet.SetFilePos(strlen("T"));
+  auto pid_tid = packet.GetPidTid(m_current_process ? 
m_current_process->GetID()
+: LLDB_INVALID_PROCESS_ID);
+  if (!pid_tid)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
+  lldb::pid_t pid = pid_tid->first;
+  lldb::tid_t tid = pid_tid->second;
+
+  // Technically, this would also be caught by the PID check but let's be more
+  // explicit about the error.
+  if (pid == LLDB_INVALID_PROCESS_ID)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "No current process and no PID provided"));
+
+  // Check the process ID and find respective process instance.
+  auto new_process_it = m_debugged_processes.find(pid);
+  if (new_process_it == m_debugged_processes.end())
+return SendErrorResponse(1);
+
+  // Check the thread ID
+  if (!new_process_it->second->GetThreadByID(tid))
+return SendErrorResponse(2);
+
+  return SendOKResponse();
+}
+
 void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() {
   Log *log = GetLog(LLDBLog::Process);
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 03a548b076e46..abb7f10f4830f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -246,6 +246,8 @@ class GDBRemoteCommunicationServerLLGS
 
   PacketResult Handle_QMemTags(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_T(StringExtractorGDBRemote &packet);
+
   void SetCurrentThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetCurrentThreadID() const;

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index fdd9e94d0a364..23e387afd4cd4 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -866,3 +866,76 @@ def test_qC(self):
  "send packet: $QCp{:x}.{:x}#00".format(*pidtid),
  ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_T(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+s

[Lldb-commits] [lldb] 4b485fc - [lldb] [llgs] Introduce an AppendThreadIDToResponse() helper

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:24+02:00
New Revision: 4b485fc0ea4acf065c7992a5c9a1223f5565544e

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

LOG: [lldb] [llgs] Introduce an AppendThreadIDToResponse() helper

Introduce a helper function to append GDB Remote Serial Protocol "thread
IDs", with optional PID in multiprocess mode.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128324

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index f5c66496d765..9356b01a6ce8 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -826,10 +826,8 @@ 
GDBRemoteCommunicationServerLLGS::PrepareStopReplyPacketForThread(
 
   // Include the (pid and) tid.
   response.PutCString("thread:");
-  if (bool(m_extensions_supported &
-   NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", process.GetID());
-  response.Format("{0:x-};", thread.GetID());
+  AppendThreadIDToResponse(response, process.GetID(), thread.GetID());
+  response.PutChar(';');
 
   // Include the thread name if there is one.
   const std::string thread_name = thread.GetName();
@@ -1425,9 +1423,8 @@ 
GDBRemoteCommunicationServerLLGS::Handle_qC(StringExtractorGDBRemote &packet) {
 
   StreamString response;
   response.PutCString("QC");
-  if (bool(m_extensions_supported & 
NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", m_current_process->GetID());
-  response.Format("{0:x-}", thread->GetID());
+  AppendThreadIDToResponse(response, m_current_process->GetID(),
+   thread->GetID());
 
   return SendPacketNoLock(response.GetString());
 }
@@ -1996,10 +1993,7 @@ void GDBRemoteCommunicationServerLLGS::AddProcessThreads(
 LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
  thread->GetID());
 response.PutChar(had_any ? ',' : 'm');
-if (bool(m_extensions_supported &
- NativeProcessProtocol::Extension::multiprocess))
-  response.Format("p{0:x-}.", pid);
-response.Format("{0:x-}", thread->GetID());
+AppendThreadIDToResponse(response, pid, thread->GetID());
 had_any = true;
   }
 }
@@ -4143,6 +4137,14 @@ 
GDBRemoteCommunicationServerLLGS::SendContinueSuccessResponse() {
   return m_non_stop ? SendOKResponse() : PacketResult::Success;
 }
 
+void GDBRemoteCommunicationServerLLGS::AppendThreadIDToResponse(
+Stream &response, lldb::pid_t pid, lldb::tid_t tid) {
+  if (bool(m_extensions_supported &
+   NativeProcessProtocol::Extension::multiprocess))
+response.Format("p{0:x-}.", pid);
+  response.Format("{0:x-}", tid);
+}
+
 std::string
 lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
bool reverse_connect) {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index abb7f10f4830..5187a953f957 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -273,6 +273,9 @@ class GDBRemoteCommunicationServerLLGS
   // in non-stop mode, no response otherwise.
   PacketResult SendContinueSuccessResponse();
 
+  void AppendThreadIDToResponse(Stream &response, lldb::pid_t pid,
+lldb::tid_t tid);
+
 private:
   llvm::Expected> BuildTargetXml();
 



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

[Lldb-commits] [lldb] c1829e0 - [lldb] [test] Move part of fork tests to common helper

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-24T17:20:25+02:00
New Revision: c1829e0ec58bd974e65639de0b9cbab736bb624f

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

LOG: [lldb] [test] Move part of fork tests to common helper

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128361

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 23e387afd4cd..d8c93b84d0ce 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -12,25 +12,31 @@ class 
TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-@add_test_categories(["fork"])
-def test_fork_multithreaded(self):
+def start_fork_test(self, args, variant="fork"):
 self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + 
["fork"])
-self.add_qSupported_packets(["multiprocess+", "fork-events+"])
+self.prep_debug_monitor_and_inferior(inferior_args=args)
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
 ret = self.expect_gdbremote_sequence()
-self.assertIn("fork-events+", ret["qSupported_response"])
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
 self.reset_test_sequence()
 
 # continue and expect fork
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format("fork"),
+{"direction": "send", "regex": self.fork_regex.format(variant),
  "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-child_pid = ret["child_pid"]
 self.reset_test_sequence()
 
+return tuple(ret[x] for x in ("parent_pid", "parent_tid",
+  "child_pid", "child_tid"))
+
+@add_test_categories(["fork"])
+def test_fork_multithreaded(self):
+_, _, child_pid, _ = self.start_fork_test(["thread:new"]*2 + ["fork"])
+
 # detach the forked child
 self.test_sequence.add_log_lines([
 "read packet: $D;{}#00".format(child_pid),
@@ -40,26 +46,8 @@ def test_fork_multithreaded(self):
 self.expect_gdbremote_sequence()
 
 def fork_and_detach_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -106,26 +94,8 @@ def test_vfork(self):
 self.expect_gdbremote_sequence()
 
 def fork_and_follow_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, chi

[Lldb-commits] [PATCH] D128150: [lldb] [llgs] Add a test for multiprocess memory read/write

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0481d8efa92f: [lldb] [llgs] Add a test for multiprocess 
memory read/write (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128150

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -553,3 +553,90 @@
 "send packet: $E03#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_memory_read_write(self):
+self.build()
+INITIAL_DATA = "Initial message"
+self.prep_debug_monitor_and_inferior(
+inferior_args=["set-message:{}".format(INITIAL_DATA),
+   "get-data-address-hex:g_message",
+   "fork",
+   "print-message:",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"type": "output_match",
+ "regex": self.maybe_strict_output_regex(r"data address: 
0x([0-9a-fA-F]+)\r\n"),
+ "capture": {1: "addr"}},
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+pidtids = {
+"parent": (ret["parent_pid"], ret["parent_tid"]),
+"child": (ret["child_pid"], ret["child_tid"]),
+}
+addr = ret["addr"]
+self.reset_test_sequence()
+
+for name, pidtid in pidtids.items():
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # read the current memory contents
+ "read packet: $m{},{:x}#00".format(addr,
+len(INITIAL_DATA) + 1),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ # write a new value
+ "read packet: $M{},{:x}:{}#00".format(addr,
+   len(name) + 1,
+   seven.hexlify(
+   name + "\0")),
+ "send packet: $OK#00",
+ # resume the process and wait for the trap
+ "read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"type": "output_match",
+  "regex": self.maybe_strict_output_regex(r"message: 
(.*)\r\n"),
+  "capture": {1: "printed_message"}},
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+ret = self.expect_gdbremote_sequence()
+data = seven.unhexlify(ret["data"])
+self.assertEqual(data, INITIAL_DATA + "\0")
+self.assertEqual(ret["printed_message"], name);
+self.reset_test_sequence()
+
+# we do the second round separately to make sure that initial data
+# is correctly preserved while writing into the first process
+
+for name, pidtid in pidtids.items():
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # read the current memory contents
+ "read packet: $m{},{:x}#00".format(addr,
+len(name) + 1),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ ], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIsNotNone(ret.get("data"))
+data = seven.unhexlify(ret.get("data"))
+self.assertEqual(data, name + "\0")
+self.reset_test_sequence()


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
=

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG75757c86c695: [lldb] [llgs] Support multiprocess in 
qfThreadInfo (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128152

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -554,6 +554,66 @@
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_threadinfo(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+prev_pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(prev_pidtids,
+ frozenset((int(pid, 16), int(tid, 16))
+   for pid, tid in pidtids))
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+new_pidtids = set(self.parse_threadinfo_packets(ret))
+added_pidtid = new_pidtids - prev_pidtids
+prev_pidtids = new_pidtids
+
+# verify that we've got exactly one new thread, and that
+# the PID matches
+self.assertEqual(len(added_pidtid), 1)
+self.assertEqual(added_pidtid.pop()[0], int(pidtid[0], 16))
+
+for pidtid in new_pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_memory_read_write(self):
 self.build()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -159,6 +159,9 @@
 
   PacketResult Handle_qRegisterInfo(StringExtractorGDBRemote &packet);
 
+  void AddProcessThreads(StreamGDBRemote &response,
+ NativeProcessProtocol &process, bool &had_any);
+
   PacketResult Handle_qfThreadInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1974,38 +1974,46 @@
   return SendPacketNoLock(response.GetString());
 }
 
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
-StringExtractorGDBRemote &packet) {
+void GDBRemoteCommunicationServerLLGS::AddProcessThreads(
+StreamGDBRemote &response, NativeProcessProtocol &process, bool &had_any) {
   Log *log = GetLog(LLDBLog::Thread);
 
-  // Fail if we don't have a current process.
-  if (!m_current_p

[Lldb-commits] [PATCH] D128153: [lldb] [llgs] Add a test for multiprocess register read/write

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG14d67073359a: [lldb] [llgs] Add a test for multiprocess 
register read/write (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128153

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -1,3 +1,5 @@
+import random
+
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -700,3 +702,118 @@
 data = seven.unhexlify(ret.get("data"))
 self.assertEqual(data, name + "\0")
 self.reset_test_sequence()
+
+@add_test_categories(["fork"])
+def test_register_read_write(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(len(pidtids), 4)
+# first, save register values from all the threads
+thread_regs = {}
+for pidtid in pidtids:
+for regno in range(256):
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $p{:x}#00".format(regno),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ ], True)
+ret = self.expect_gdbremote_sequence()
+data = ret.get("data")
+self.assertIsNotNone(data)
+# ignore registers shorter than 32 bits (this also catches
+# "Exx" errors)
+if len(data) >= 8:
+break
+else:
+self.skipTest("no usable register found")
+thread_regs[pidtid] = (regno, data)
+
+vals = set(x[1] for x in thread_regs.values())
+# NB: cheap hack to make the loop below easier
+new_val = next(iter(vals))
+
+# then, start altering them and verify that we don't unexpectedly
+# change the value from another thread
+for pidtid in pidtids:
+old_val = thread_regs[pidtid]
+regno = old_val[0]
+old_val_length = len(old_val[1])
+# generate a unique new_val
+while new_val in vals:
+new_val = ('{{:0{}x}}'.format(old_val_length)
+   .format(random.getrandbits(old_val_length*4)))
+vals.add(new_val)
+
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $p{:x}#00".format(regno),
+ {"direction": "send",
+  "regex": r"^[$](.+)#.*$",
+  "capture": {1: "data"}},
+ "read packet: $P{:x}={}#00".format(regno, new_val),
+ "send packet: $OK#00",
+ ], True)
+ret = self.expect_gdbremote_sequence()
+data = ret.get("data")
+self.assertIsN

[Lldb-commits] [PATCH] D128156: [lldb] [llgs] Include PID in QC response in multiprocess mode

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG630da0e309ef: [lldb] [llgs] Include PID in QC response in 
multiprocess mode (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128156

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -11,7 +11,6 @@
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
-procinfo_regex = "[$]pid:([0-9a-f]+);.*"
 
 @add_test_categories(["fork"])
 def test_fork_multithreaded(self):
@@ -68,7 +67,7 @@
 "send packet: $OK#00",
 # verify that the current process is correct
 "read packet: $qC#00",
-"send packet: $QC{}#00".format(parent_tid),
+"send packet: $QCp{}.{}#00".format(parent_pid, parent_tid),
 # verify that the correct processes are detached/available
 "read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
 "send packet: $Eff#00",
@@ -167,12 +166,9 @@
 
 # get process pid
 self.test_sequence.add_log_lines([
-"read packet: $qProcessInfo#00",
-{"direction": "send", "regex": self.procinfo_regex,
- "capture": {1: "pid"}},
 "read packet: $qC#00",
-{"direction": "send", "regex": "[$]QC([0-9a-f]+)#.*",
- "capture": {1: "tid"}},
+{"direction": "send", "regex": "[$]QCp([0-9a-f]+).([0-9a-f]+)#.*",
+ "capture": {1: "pid", 2: "tid"}},
 ], True)
 ret = self.expect_gdbremote_sequence()
 pid, tid = (int(ret[x], 16) for x in ("pid", "tid"))
@@ -208,8 +204,8 @@
 
 # get process pid
 self.test_sequence.add_log_lines([
-"read packet: $qProcessInfo#00",
-{"direction": "send", "regex": self.procinfo_regex,
+"read packet: $qC#00",
+{"direction": "send", "regex": "[$]QCp([0-9a-f]+).[0-9a-f]+#.*",
  "capture": {1: "pid"}},
 ], True)
 ret = self.expect_gdbremote_sequence()
@@ -817,3 +813,56 @@
 data = ret.get("data")
 self.assertIsNotNone(data)
 self.assertEqual(data, old_val[1])
+
+@add_test_categories(["fork"])
+def test_qC(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(len(pidtids), 4)
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $qC#00",
+ "send packet: $QCp{:x}.{:x}#00".format(*pidtid),
+ ], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Pr

[Lldb-commits] [PATCH] D128170: [lldb] [llgs] Implement the 'T' packet

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe827e5186fb6: [lldb] [llgs] Implement the 'T' 
packet (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128170

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -866,3 +866,76 @@
  "send packet: $QCp{:x}.{:x}#00".format(*pidtid),
  ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_T(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(len(pidtids), 4)
+max_pid = max(pid for pid, tid in pidtids)
+max_tid = max(tid for pid, tid in pidtids)
+bad_pidtids = (
+(max_pid, max_tid + 1, "E02"),
+(max_pid + 1, max_tid, "E01"),
+(max_pid + 1, max_tid + 1, "E01"),
+)
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+[
+ # test explicit PID+TID
+ "read packet: $Tp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # test implicit PID via Hg
+ "read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $T{:x}#00".format(max_tid + 1),
+ "send packet: $E02#00",
+ "read packet: $T{:x}#00".format(pidtid[1]),
+ "send packet: $OK#00",
+ ], True)
+for pid, tid, expected in bad_pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Tp{:x}.{:x}#00".format(pid, tid),
+ "send packet: ${}#00".format(expected),
+ ], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -246,6 +246,8 @@
 
   PacketResult Handle_QMemTags(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_T(StringExtractorGDBRemote &packet);
+
   void SetCurrentThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetCurrentThreadID() const;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -107,6 +107,8 @@
 &GDBRemoteCommunicationServerLLGS::Handle_P);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qC,
 &GDBRemoteCommunicationSer

[Lldb-commits] [PATCH] D128324: [lldb] [llgs] Introduce an AppendThreadIDToResponse() helper

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b485fc0ea4a: [lldb] [llgs] Introduce an 
AppendThreadIDToResponse() helper (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128324

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -273,6 +273,9 @@
   // in non-stop mode, no response otherwise.
   PacketResult SendContinueSuccessResponse();
 
+  void AppendThreadIDToResponse(Stream &response, lldb::pid_t pid,
+lldb::tid_t tid);
+
 private:
   llvm::Expected> BuildTargetXml();
 
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -826,10 +826,8 @@
 
   // Include the (pid and) tid.
   response.PutCString("thread:");
-  if (bool(m_extensions_supported &
-   NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", process.GetID());
-  response.Format("{0:x-};", thread.GetID());
+  AppendThreadIDToResponse(response, process.GetID(), thread.GetID());
+  response.PutChar(';');
 
   // Include the thread name if there is one.
   const std::string thread_name = thread.GetName();
@@ -1425,9 +1423,8 @@
 
   StreamString response;
   response.PutCString("QC");
-  if (bool(m_extensions_supported & 
NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", m_current_process->GetID());
-  response.Format("{0:x-}", thread->GetID());
+  AppendThreadIDToResponse(response, m_current_process->GetID(),
+   thread->GetID());
 
   return SendPacketNoLock(response.GetString());
 }
@@ -1996,10 +1993,7 @@
 LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
  thread->GetID());
 response.PutChar(had_any ? ',' : 'm');
-if (bool(m_extensions_supported &
- NativeProcessProtocol::Extension::multiprocess))
-  response.Format("p{0:x-}.", pid);
-response.Format("{0:x-}", thread->GetID());
+AppendThreadIDToResponse(response, pid, thread->GetID());
 had_any = true;
   }
 }
@@ -4143,6 +4137,14 @@
   return m_non_stop ? SendOKResponse() : PacketResult::Success;
 }
 
+void GDBRemoteCommunicationServerLLGS::AppendThreadIDToResponse(
+Stream &response, lldb::pid_t pid, lldb::tid_t tid) {
+  if (bool(m_extensions_supported &
+   NativeProcessProtocol::Extension::multiprocess))
+response.Format("p{0:x-}.", pid);
+  response.Format("{0:x-}", tid);
+}
+
 std::string
 lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
bool reverse_connect) {


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -273,6 +273,9 @@
   // in non-stop mode, no response otherwise.
   PacketResult SendContinueSuccessResponse();
 
+  void AppendThreadIDToResponse(Stream &response, lldb::pid_t pid,
+lldb::tid_t tid);
+
 private:
   llvm::Expected> BuildTargetXml();
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -826,10 +826,8 @@
 
   // Include the (pid and) tid.
   response.PutCString("thread:");
-  if (bool(m_extensions_supported &
-   NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", process.GetID());
-  response.Format("{0:x-};", thread.GetID());
+  AppendThreadIDToResponse(response, process.GetID(), thread.GetID());
+  response.PutChar(';');
 
   // Include the thread name if there is one.
   const std::string thread_name = thread.GetName();
@@ -1425,9 +1423,8 @@
 
   StreamString response;
   response.PutCString("QC");
-  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", m_current_process->GetID());
-  response.Format("

[Lldb-commits] [PATCH] D128361: [lldb] [test] Move part of fork tests to common helper

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc1829e0ec58b: [lldb] [test] Move part of fork tests to 
common helper (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128361

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -12,25 +12,31 @@
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-@add_test_categories(["fork"])
-def test_fork_multithreaded(self):
+def start_fork_test(self, args, variant="fork"):
 self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + ["fork"])
-self.add_qSupported_packets(["multiprocess+", "fork-events+"])
+self.prep_debug_monitor_and_inferior(inferior_args=args)
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
 ret = self.expect_gdbremote_sequence()
-self.assertIn("fork-events+", ret["qSupported_response"])
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
 self.reset_test_sequence()
 
 # continue and expect fork
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format("fork"),
+{"direction": "send", "regex": self.fork_regex.format(variant),
  "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-child_pid = ret["child_pid"]
 self.reset_test_sequence()
 
+return tuple(ret[x] for x in ("parent_pid", "parent_tid",
+  "child_pid", "child_tid"))
+
+@add_test_categories(["fork"])
+def test_fork_multithreaded(self):
+_, _, child_pid, _ = self.start_fork_test(["thread:new"]*2 + ["fork"])
+
 # detach the forked child
 self.test_sequence.add_log_lines([
 "read packet: $D;{}#00".format(child_pid),
@@ -40,26 +46,8 @@
 self.expect_gdbremote_sequence()
 
 def fork_and_detach_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -106,26 +94,8 @@
 self.expect_gdbremote_sequence()
 
 def fork_and_follow_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant))
 
 # switch to the forked child
 self.test_sequence.add_log_lines([
@@ -223,26 +193,8 @@
 
 @add_test_categories(["fork"])
 def test_detach_all(self):
-self.build()
-self.prep_debug_monitor_and_inferior(infer

[Lldb-commits] [PATCH] D128484: [NFC][lldb][trace] Rename trace session to trace bundle

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb8dcd0ba26a9: [NFC][lldb][trace] Rename trace session to 
trace bundle (authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D128484?vs=439582&id=439783#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128484

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/Trace/common/ThreadPostMortemTrace.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -98,7 +98,7 @@
 
 # We test first an invalid type
 trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace_bad.json")
-expected_substrs = ['''error: expected object at traceSession.processes[0]
+expected_substrs = ['''error: expected object at traceBundle.processes[0]
 
 Context:
 {
@@ -124,15 +124,15 @@
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, error=True, substrs=expected_substrs)
 
 
-# Now we test a wrong cpu family field in the global session file
+# Now we test a wrong cpu family field in the global bundle description file
 trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace_bad2.json")
-expected_substrs = ['error: expected uint64_t at traceSession.cpuInfo.family', "Context", "Schema"]
+expected_substrs = ['error: expected uint64_t at traceBundle.cpuInfo.family', "Context", "Schema"]
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, error=True, substrs=expected_substrs)
 
 
 # Now we test a missing field in the intel-pt settings
 trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace_bad4.json")
-expected_substrs = ['''error: missing value at traceSession.cpuInfo.family
+expected_substrs = ['''error: missing value at traceBundle.cpuInfo.family
 
 Context:
 {
@@ -149,7 +149,7 @@
 
 # Now we test an incorrect load address in the intel-pt settings
 trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace_bad5.json")
-expected_substrs = ['error: missing value at traceSession.processes[1].pid', "Schema"]
+expected_substrs = ['error: missing value at traceBundle.processes[1].pid', "Schema"]
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, error=True, substrs=expected_substrs)
 
 
@@ -157,6 +157,6 @@
 # no targets should be created.
 self.assertEqual(self.dbg.GetNumTargets(), 0)
 trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace_bad3.json")
-expected_substrs = ['error: missing value at traceSession.processes[1].pid']
+expected_substrs = ['error: missing value at traceBundle.processes[1].pid']
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, error=True, substrs=expected_substrs)
 self.assertEqual(self.dbg.GetNumTargets(), 0)
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -24,19 +24,20 @@
 using namespace lldb_private;
 using namespace llvm;
 
-// Helper structs used to extract the type of a trace session json without
-// having to parse the entire object.
+// Helper structs used to extract the type of a JSON trace bundle description
+// object without having to parse the entire object.
 
-struct JSONSimpleTraceSession {
+struct JSONSimpleTraceBundleDescription {
   std::string type;
 };
 
 namespace llvm {
 namespace json {
 
-

[Lldb-commits] [lldb] b8dcd0b - [NFC][lldb][trace] Rename trace session to trace bundle

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-24T08:41:33-07:00
New Revision: b8dcd0ba26a90166cc76e6d1db23b88656610b2e

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

LOG: [NFC][lldb][trace] Rename trace session to trace bundle

As previously discussed with @jj10306, we didn't really have a name for
the post-mortem (or offline) trace session representation, which is in
fact a folder with a bunch of files. We decided to call this folder
"trace bundle", and the main JSON file in it "trace bundle description
file". This naming is pretty decent, so I'm refactoring all the existing
code to account for that.

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

Added: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.h

Modified: 
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Commands/CommandObjectTrace.cpp
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/Trace/common/ThreadPostMortemTrace.h
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h



diff  --git a/lldb/include/lldb/Core/PluginManager.h 
b/lldb/include/lldb/Core/PluginManager.h
index 8322585b2253e..58be1d2b18dcc 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -336,21 +336,21 @@ class PluginManager {
   // Trace
   static bool RegisterPlugin(
   llvm::StringRef name, llvm::StringRef description,
-  TraceCreateInstanceForSessionFile create_callback_for_session_file,
+  TraceCreateInstanceFromBundle create_callback_from_bundle,
   TraceCreateInstanceForLiveProcess create_callback_for_live_process,
   llvm::StringRef schema);
 
   static bool
-  UnregisterPlugin(TraceCreateInstanceForSessionFile create_callback);
+  UnregisterPlugin(TraceCreateInstanceFromBundle create_callback);
 
-  static TraceCreateInstanceForSessionFile
+  static TraceCreateInstanceFromBundle
   GetTraceCreateCallback(llvm::StringRef plugin_name);
 
   static TraceCreateInstanceForLiveProcess
   GetTraceCreateCallbackForLiveProcess(llvm::StringRef plugin_name);
 
-  /// Get the JSON schema for a trace session file corresponding to the given
-  /// plugin.
+  /// Get the JSON schema for a trace bundle description file corresponding to
+  /// the given plugin.
   ///
   /// \param[in] plugin_name
   /// The name of the plugin.
@@ -360,8 +360,8 @@ class PluginManager {
   /// otherwise the actual schema is returned.
   static llvm::StringRef GetTraceSchema(llvm::StringRef plugin_name);
 
-  /// Get the JSON schema for a trace session file corresponding to the plugin
-  /// given by its index.
+  /// Get the JSON schema for a trace bundle description file corresponding to
+  /// the plugin given by its index.
   ///
   /// \param[in] index
   /// The index of the plugin to get the schema of.

diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 216394bdd1847..6e06ceb260e5f 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -102,17 +102,14 @@ class Trace : public PluginInterface,
   /// The debugger instance where new Targets will be created as part of 
the
   /// JSON data parsing.
   ///
-  /// \param[in] trace_session_file
-  /// The contents of the trace session file describing the trace session.
-  /// See \a TraceSessionFileParser::BuildSchema for more information about
-  /// the schema of this JSON file.
-  ///
-  /// \param[in] session_file_dir
-  /// The path to the directory that contains the session file. It's used 
to
-  /// resolved relative paths in the session file.
+  /// \param[in] bundle_description
+  /// The trace bundle description object describing the trace session.
+  ///
+  /// \param[in] bundle_dir
+  /// The path to the directory that contains the trace bundle.
   static l

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Host/common/Host.cpp:110
+#if !defined(__APPLE__)
+void Host::SystemLog(const std::string &message) {
+  fprintf(stderr, "%s", message.c_str());

labath wrote:
> labath wrote:
> > Why std::string? I'd expect StringRef (as that's what the LogHandler class 
> > uses), const char * (as that can avoid string copying sometimes) or a Twine 
> > (in case you want to be fancy), but a string seems like it combines the 
> > worst properties of all of these.
> Actually, I think we should just use a StringRef and use llvm::errs() for 
> printing.
Right, I was optimizing for the Apple case where we need to go through a 
`std::string` to make sure it's null terminated. My reasoning was that if you 
pass in a StringRef you might end up with two copies if the original StringRef 
is already backed by a std::string.


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

https://reviews.llvm.org/D128321

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

Re: [Lldb-commits] [PATCH] D128504: debugserver: spawn process in its own process group

[Jim Ingham via lldb-commits]

Why is it desirable to have the debugger not see signals sent to the process?

Jim


> On Jun 24, 2022, at 1:06 AM, Alessandro Arzilli via Phabricator via 
> lldb-commits  wrote:
> 
> aarzilli created this revision.
> aarzilli added reviewers: jasonmolenda, clayborg.
> aarzilli added a project: LLDB.
> Herald added a subscriber: JDevlieghere.
> Herald added a project: All.
> aarzilli requested review of this revision.
> Herald added a subscriber: lldb-commits.
> 
> Change POSIX spawn launch flavor to spawn process in its own process group, 
> like the fork/exec flavor does.
> This is useful because the target process can then be made controlling 
> process for its tty and receive terminal signals (such as SIGINT generated in 
> response to the user pressing ^C) without debugserver also receiving them.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> https://reviews.llvm.org/D128504
> 
> Files:
>  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> 
> 
> Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> ===
> --- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> +++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> @@ -3268,7 +3268,7 @@
> return INVALID_NUB_PROCESS;
> 
>   flags = POSIX_SPAWN_START_SUSPENDED | POSIX_SPAWN_SETSIGDEF |
> -  POSIX_SPAWN_SETSIGMASK;
> +  POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETPGROUP;
>   if (disable_aslr)
> flags |= _POSIX_SPAWN_DISABLE_ASLR;
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D128504: debugserver: spawn process in its own process group

[Alessandro Arzilli via Phabricator via lldb-commits]

aarzilli added a comment.

In D128504#3608530 , @jingham wrote:

> Why is it desirable to have the debugger not see signals sent to the process?
>
> Jim

Regardless of this patch, it is always possible to send signals directly to the 
target process. This is only about signals generated by the terminal.
Suppose you are debugging a program that does something in response to ^C: you 
will have to use kill in a different terminal to send the signal to the target 
process, because pressing ^C will send a signal to both debugserver and the 
target process, causing debugserver to die in response. I think it should be 
possible to set up the target process so that it has full control of its 
terminal, as it would be if it had been run outside of a debugger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128504

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

@jingham

Sorry for spamming. But I am not sure if you have received my previous messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [lldb] 6879391 - [lldb] Replace Host::SystemLog with Debugger::Report{Error, Warning}

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-24T09:46:26-07:00
New Revision: 6879391908cab68e8d43a0097fa8c9cd3b86eff9

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

LOG: [lldb] Replace Host::SystemLog with Debugger::Report{Error,Warning}

As it exists today, Host::SystemLog is used exclusively for error
reporting. With the introduction of diagnostic events, we have a better
way of reporting those. Instead of printing directly to stderr, these
messages now get printed to the debugger's error stream (when using the
default event handler). Alternatively, if someone is listening for these
events, they can decide how to display them, for example in the context
of an IDE such as Xcode.

This change also means we no longer write these messages to the system
log on Darwin. As far as I know, nobody is relying on this, but I think
this is something we could add to the diagnostic event mechanism.

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

Added: 


Modified: 
lldb/include/lldb/Host/Host.h
lldb/source/Core/Module.cpp
lldb/source/Host/common/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Interpreter/Options.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Symbol/CompactUnwindInfo.cpp
lldb/source/Symbol/DWARFCallFrameInfo.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/SymbolContext.cpp

lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
lldb/tools/lldb-server/lldb-platform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 3b49b5e393507..b0097a1c1c420 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -86,13 +86,6 @@ class Host {
   StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
   lldb::pid_t pid);
 
-  enum SystemLogType { eSystemLogWarning, eSystemLogError };
-
-  static void SystemLog(SystemLogType type, const char *format, ...)
-  __attribute__((format(printf, 2, 3)));
-
-  static void SystemLog(SystemLogType type, const char *format, va_list args);
-
   /// Get the process ID for the calling process.
   ///
   /// \return

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 7160c2386efd6..41c21e1dc326b 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1107,27 +1107,6 @@ void Module::GetDescription(llvm::raw_ostream &s,
 s << llvm::formatv("({0})", object_name);
 }
 
-void Module::ReportError(const char *format, ...) {
-  if (format && format[0]) {
-StreamString strm;
-strm.PutCString("error: ");
-GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
-strm.PutChar(' ');
-va_list args;
-va_start(args, format);
-strm.PrintfVarArg(format, args);
-va_end(args);
-
-const int format_len = strlen(format);
-if (format_len > 0) {
-  const char last_char = format[format_len - 1];
-  if (last_char != '\n' && last_char != '\r')
-strm.EOL();
-}
-Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
-  }
-}
-
 bool Module::FileHasChanged() const {
   // We have provided the DataBuffer for this module to avoid accessing the
   // filesystem. We never want to reload those files.
@@ -1170,7 +1149,7 @@ void Module::ReportErrorIfModifyDetected(const char 
*format, ...) {
   m_first_file_changed_log = true;
   if (format) {
 StreamString strm;
-strm.PutCString("error: the object file ");
+strm.PutCString("the object file ");
 GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
 strm.PutCString(" has been modified\n");
 
@@ -1186,17 +1165,31 @@ void Module::ReportErrorIfModifyDetected(const char 
*format, ...) {
 strm.EOL();
 }
 strm.PutCString("The debug session should be aborted as the original "
-"debug information has been overwritten.\n");
-Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
+"debug information has been overwritten.");
+Debugger::ReportError(std::string(strm.GetString()));
   }
 }
   }
 }
 
+void Module::ReportError(const char *format, ...) {
+  if (format && format[0]) {
+StreamString strm;
+GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
+strm.PutChar(' ');
+
+va_list args;
+va_start(args, format);
+strm.PrintfVarArg(format, args);
+va_end(args);
+
+Debugger::ReportError(std::string(strm.GetString()));
+  }
+}
+
 void Module::ReportWarning(const char *format, ...) {
   if (format && form

[Lldb-commits] [PATCH] D128480: [lldb] Replace Host::SystemLog with Debugger::Report{Error, Warning}

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6879391908ca: [lldb] Replace Host::SystemLog with 
Debugger::Report{Error,Warning} (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D128480?vs=439577&id=439803#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128480

Files:
  lldb/include/lldb/Host/Host.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Interpreter/Options.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/CompactUnwindInfo.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
  lldb/tools/lldb-server/lldb-platform.cpp

Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -77,10 +77,9 @@
   case SIGHUP:
 // Use SIGINT first, if that does not work, use SIGHUP as a last resort.
 // And we should not call exit() here because it results in the global
-// destructors
-// to be invoked and wreaking havoc on the threads still running.
-Host::SystemLog(Host::eSystemLogWarning,
-"SIGHUP received, exiting lldb-server...\n");
+// destructors to be invoked and wreaking havoc on the threads still
+// running.
+llvm::errs() << "SIGHUP received, exiting lldb-server...\n";
 abort();
 break;
   }
Index: lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
===
--- lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
+++ lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
@@ -30,12 +30,15 @@
 self.assertTrue(os.path.isdir(mod_cache), "module cache exists")
 
 logfile = self.getBuildArtifact("host.log")
-self.runCmd("log enable -v -f %s lldb host" % logfile)
-target, _, _, _ = lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.m"))
-target.GetModuleAtIndex(0).FindTypes('my_int')
+with open(logfile, 'w') as f:
+sbf = lldb.SBFile(f.fileno(), 'w', False)
+status = self.dbg.SetErrorFile(sbf)
+self.assertSuccess(status)
+
+target, _, _, _ = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.m"))
+target.GetModuleAtIndex(0).FindTypes('my_int')
 
-found = False
 with open(logfile, 'r') as f:
 for line in f:
 if "hash mismatch" in line and "Foo" in line:
Index: lldb/source/Symbol/SymbolContext.cpp
===
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Symbol/SymbolContext.h"
 
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
@@ -494,20 +495,16 @@
   objfile = symbol_file->GetObjectFile();
   }
   if (objfile) {
-Host::SystemLog(
-Host::eSystemLogWarning,
-"warning: inlined block 0x%8.8" PRIx64
-" doesn't have a range that contains file address 0x%" PRIx64
-" in %s\n",
+Debugger::ReportWarning(llvm::formatv(
+"inlined block {0:x} doesn't have a range that contains file "
+"address {1:x} in {2}",
 curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress(),
-objfile->GetFileSpec().GetPath().c_str());
+objfile->GetFileSpec().GetPath()));
   } else {
-Host::SystemLog(
-Host::eSystemLogWarning,
-"warning: inlined block 0x%8.8" PRIx64
-" doesn't have a range that contains file address 0x%" PRIx64
-"\n",
-curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress());
+Debugger::ReportWarning(llvm::formatv(
+"inlined block {0:x} doesn't have a range that contains file "
+"address {1:x}",
+curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress()));
   }
 }
 #endif
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -7,6 +7,7 @@
 //===-

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

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

In D128321#3607935 , @labath wrote:

> In D128321#3605592 , @JDevlieghere 
> wrote:
>
>> I also considered that, but that just delays the problem until I want to 
>> enable this handler for the always-on logging (as outlined in 
>> https://discourse.llvm.org/c/subprojects/lldb/8). I haven't really looked 
>> into where that logic would live, but the reproducers lived in Utility so I 
>> was expecting all of this to be there as well.
>
> I don't think that's a particularly big problem. Any code that's needed to 
> enable always-on logging (to /a/ log handler) can stay in Utility. Only the 
> code which actually enables it (and passes a specific log handler) needs to 
> live outside. One way to do that would be to put something in the 
> SystemInitializer class (like we did for reproducers). Another would be to 
> use the shiny new global lldbinit file feature (which would allow site 
> customization). :P

Right, not sure why I didn't think of that myself. Alright that covers all my 
concerns. Given that this was Greg's suggestion I'm assuming he's happy with 
this as well. Thank you both for the review!


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

@labath Any chance I can nerd-snipe you into adding support for syslog (or 
whatever the modern equivalent is) on linux? :-)


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 439814.
zequanwu marked an inline comment as done.
zequanwu added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Expression/DWARFExpressionList.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Utility/RangeMap.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/DWARFExpressionList.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
@@ -23,6 +23,7 @@
   include_dirs = [ ".." ]
   sources = [
 "DWARFExpression.cpp",
+"DWARFExpressionList.cpp",
 "DiagnosticManager.cpp",
 "Expression.cpp",
 "ExpressionVariable.cpp",
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -19,29 +19,29 @@
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x0
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F0
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0001}, language = "", file = '1.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x2
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F1
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0001-0x0002)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0003, 0x0001): DW_OP_reg1 RDX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0001, 0x0002): DW_OP_reg1 RDX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0002}, language = "", file = '2.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x4
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F2
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0002-0x0003)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0005, 0x0001): DW_OP_reg2 RCX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0002, 0x0003): DW_OP_reg2 RCX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0003}, language = "", file = '3.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x6
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F3
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0003-0x0004)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0007, 0x0001): DW_OP_reg3 RBX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0003, 0x0004): DW_OP_reg3 RBX

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

In D50304#3592849 , @RamNalamothu 
wrote:

> In D50304#3586710 , @jingham wrote:
>
>> For some reason I'm not getting mail notifications for review changes, sorry 
>> about that.
>>
>> This is certainly better than the original implementation.  Among other 
>> things, if we find an exact match, we really shouldn't be doing any more 
>> inexact matches, so after the first exact hit it should have switched exact 
>> to true.
>>
>> But if you had line tables laid out like (this is in increasing order of 
>> address:
>>
>> 10
>> 20
>> 30
>> 10
>> 16
>>
>> and you did "thread until 15", this would find the inexact match at 20, 
>> switch to an exact match for line 20 and find no other matches.  But the gap 
>> between 10 & 16 in the line table is maybe an even more plausible place to 
>> put the line 15 until breakpoint, so maybe we did want to throw a breakpoint 
>> there as well?
>
> @jingham 
> Nope, with the current patch, we would find the inexact match at 16.

Ah, my bad.  FindLineEntry does end up in FindLineEntryByFileIndexImpl which 
does the closest match.

>> Regular breakpoint setting has to move inexact breakpoints in much the same 
>> way.  The code in the BreakpointResolverFileLine::SearchCallback ends up 
>> calling CompileUnit::ResolveSymbolContext to get the "best" inexact match.  
>> Maybe it would be better to not do this by hand here in the Until command, 
>> but reuse the code that we use to move break points more generally?
>
> Be it CompileUnit::ResolveSymbolContext or CompileUnit::FindLineEntry, we end 
> up calling LineTable::FindLineEntryIndexByFileIndex which finds "Exact match 
> always wins.  Otherwise try to find the closest line > the desired line." 
> when we pass `exact = false` to it.
> Given that and we anyway have to extract address list, from what we get out 
> of CompileUnit::ResolveSymbolContext, for the thread until thread plan to 
> work with, probably the additional overhead to use 
> CompileUnit::ResolveSymbolContext here does not make sense. Or am I missing 
> something?

Finding these addresses happens once in response to a user command, so I highly 
doubt that the overhead matters one way or the other.  Both end up doing the 
closest match (I misremembered where that was done) which is the main thing.  
ResolveSymbolContext does more work to handle inlined files, but there's no way 
to do "thread until" from a main CU into inlined source so that doesn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [lldb] 5a08280 - [lldb] Fix flakiness in shell tests that mixed stderr and stdout

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-24T10:53:15-07:00
New Revision: 5a08280659125b6196e1ca83d5d9d6ba412674fe

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

LOG: [lldb] Fix flakiness in shell tests that mixed stderr and stdout

Because the diagnostic events are processed by the default event handler
in its own thread, tests cannot rely on output ordering. Split stdout
and stderr to make the test reliable again.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s 
b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
index 5c1f6c46f7391..6fd2333c22e70 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -18,18 +18,20 @@
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 %s > %t-rnglistx
 # RUN: %lldb %t-rnglistx -o "image lookup -v -s lookup_rnglists" \
-# RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTX %s
+# RUN:   -o exit 2>%t.error | FileCheck --check-prefix=RNGLISTX %s
+# RUN: cat %t.error | FileCheck --check-prefix=ERROR %s
 
 # RNGLISTX-LABEL: image lookup -v -s lookup_rnglists
-# RNGLISTX: error: {{.*}} {0x003f}: DIE has DW_AT_ranges(DW_FORM_rnglistx 
0x0) attribute, but range extraction failed (DW_FORM_rnglistx cannot be used 
without DW_AT_rnglists_base for CU at 0x), please file a bug and attach 
the file at the start of this error message
+# ERROR: error: {{.*}} {0x003f}: DIE has DW_AT_ranges(DW_FORM_rnglistx 
0x0) attribute, but range extraction failed (DW_FORM_rnglistx cannot be used 
without DW_AT_rnglists_base for CU at 0x), please file a bug and attach 
the file at the start of this error message
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 --defsym RNGLISTBASE=0 %s > %t-rnglistbase
 # RUN: %lldb %t-rnglistbase -o "image lookup -v -s lookup_rnglists" \
-# RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTBASE %s
+# RUN:   -o exit 2>%t.error | FileCheck --check-prefix=RNGLISTBASE %s
+# RUN: cat %t.error | FileCheck --check-prefix=ERRORBASE %s
 
 # RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
-# RNGLISTBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has 
DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed 
(invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base 
is 24), please file a bug and attach the file at the start of this error message
+# ERRORBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has 
DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed 
(invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base 
is 24), please file a bug and attach the file at the start of this error message
 
 .text
 rnglists:

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
index ce126665b9234..4e98d99467e60 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
@@ -1,7 +1,8 @@
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit 2>&1 | FileCheck 
%s
+# RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit 2>%t.error | 
FileCheck %s
+# RUN: cat %t.error | FileCheck %s --check-prefix ERROR
 
-# CHECK: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x47) attribute, but range 
extraction failed (No debug_ranges section),
+# ERROR: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x47) attribute, but range 
extraction failed (No debug_ranges section),
 # CHECK:  Function: id = {0x001c}, name = "ranges", range = 
[0x-0x0004)
 # CHECK:Blocks: id = {0x001c}, range = [0x-0x0004)
 



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

[Lldb-commits] [lldb] 1e5d526 - [lldb] Add SystemLogHandler for emitting log messages to the system log

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-24T10:53:15-07:00
New Revision: 1e5d5261e2b6637011a65c929828ca8cb0ab8e2e

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

LOG: [lldb] Add SystemLogHandler for emitting log messages to the system log

Add a system log handler that emits log messages to the operating system
log. In addition to the log handler itself, this patch also introduces a
new Host::SystemLog helper function to abstract over writing to the
system log.

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

Added: 


Modified: 
lldb/include/lldb/Host/Host.h
lldb/source/Host/common/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm

Removed: 




diff  --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index b0097a1c1c420..b3e0cbaff19ed 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/HostThread.h"
 #include "lldb/Utility/Environment.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-private.h"
@@ -86,6 +87,9 @@ class Host {
   StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
   lldb::pid_t pid);
 
+  /// Emit the given message to the operating system log.
+  static void SystemLog(llvm::StringRef message);
+
   /// Get the process ID for the calling process.
   ///
   /// \return
@@ -252,6 +256,13 @@ class Host {
 ProcessInstanceInfoList &proc_infos);
 };
 
+/// Log handler that emits log messages to the operating system log.
+class SystemLogHandler : public LogHandler {
+public:
+  SystemLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+
 } // namespace lldb_private
 
 namespace llvm {

diff  --git a/lldb/source/Host/common/Host.cpp 
b/lldb/source/Host/common/Host.cpp
index 331042590ee92..925ed956f09b4 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -106,6 +106,10 @@ llvm::Expected 
Host::StartMonitoringChildProcess(
   });
 }
 
+#if !defined(__APPLE__)
+void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
+#endif
+
 #ifndef __linux__
 // Scoped class that will disable thread canceling when it is constructed, and
 // exception safely restore the previous value it when it goes out of scope.
@@ -627,3 +631,9 @@ uint32_t Host::FindProcesses(const ProcessInstanceInfoMatch 
&match_info,
 
   return result;
 }
+
+SystemLogHandler::SystemLogHandler() {}
+
+void SystemLogHandler::Emit(llvm::StringRef message) {
+  Host::SystemLog(message);
+}

diff  --git a/lldb/source/Host/macosx/objcxx/Host.mm 
b/lldb/source/Host/macosx/objcxx/Host.mm
index 4c369e9ec8a17..1c990c8dd16b6 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -82,6 +82,7 @@
 #include "../cfcpp/CFCString.h"
 
 #include 
+#include 
 
 #include 
 #include 
@@ -98,6 +99,20 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+
+void Host::SystemLog(llvm::StringRef message) {
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+std::call_once(g_os_log_once, []() {
+  g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+});
+os_log(g_os_log, "%{public}s", message.str().c_str());
+  } else {
+llvm::errs() << message;
+  }
+}
+
 bool Host::GetBundleDirectory(const FileSpec &file,
   FileSpec &bundle_directory) {
 #if defined(__APPLE__)



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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e5d5261e2b6: [lldb] Add SystemLogHandler for emitting log 
messages to the system log (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D128321?vs=439588&id=439820#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128321

Files:
  lldb/include/lldb/Host/Host.h
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -82,6 +82,7 @@
 #include "../cfcpp/CFCString.h"
 
 #include 
+#include 
 
 #include 
 #include 
@@ -98,6 +99,20 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+
+void Host::SystemLog(llvm::StringRef message) {
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+std::call_once(g_os_log_once, []() {
+  g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+});
+os_log(g_os_log, "%{public}s", message.str().c_str());
+  } else {
+llvm::errs() << message;
+  }
+}
+
 bool Host::GetBundleDirectory(const FileSpec &file,
   FileSpec &bundle_directory) {
 #if defined(__APPLE__)
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -106,6 +106,10 @@
   });
 }
 
+#if !defined(__APPLE__)
+void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
+#endif
+
 #ifndef __linux__
 // Scoped class that will disable thread canceling when it is constructed, and
 // exception safely restore the previous value it when it goes out of scope.
@@ -627,3 +631,9 @@
 
   return result;
 }
+
+SystemLogHandler::SystemLogHandler() {}
+
+void SystemLogHandler::Emit(llvm::StringRef message) {
+  Host::SystemLog(message);
+}
Index: lldb/include/lldb/Host/Host.h
===
--- lldb/include/lldb/Host/Host.h
+++ lldb/include/lldb/Host/Host.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/HostThread.h"
 #include "lldb/Utility/Environment.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-private.h"
@@ -86,6 +87,9 @@
   StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
   lldb::pid_t pid);
 
+  /// Emit the given message to the operating system log.
+  static void SystemLog(llvm::StringRef message);
+
   /// Get the process ID for the calling process.
   ///
   /// \return
@@ -252,6 +256,13 @@
 ProcessInstanceInfoList &proc_infos);
 };
 
+/// Log handler that emits log messages to the operating system log.
+class SystemLogHandler : public LogHandler {
+public:
+  SystemLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+
 } // namespace lldb_private
 
 namespace llvm {


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -82,6 +82,7 @@
 #include "../cfcpp/CFCString.h"
 
 #include 
+#include 
 
 #include 
 #include 
@@ -98,6 +99,20 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+
+void Host::SystemLog(llvm::StringRef message) {
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+std::call_once(g_os_log_once, []() {
+  g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+});
+os_log(g_os_log, "%{public}s", message.str().c_str());
+  } else {
+llvm::errs() << message;
+  }
+}
+
 bool Host::GetBundleDirectory(const FileSpec &file,
   FileSpec &bundle_directory) {
 #if defined(__APPLE__)
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -106,6 +106,10 @@
   });
 }
 
+#if !defined(__APPLE__)
+void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
+#endif
+
 #ifndef __linux__
 // Scoped class that will disable thread canceling when it is constructed, and
 // exception safely restore the previous value it when it goes out of scope.
@@ -627,3 +631,9 @@
 
   return result;
 }
+
+SystemLogHandler::SystemLogHandler() {}
+
+void SystemLogHandler::Emit(llvm::StringRef message) {
+  Host::SystemLog(message);
+}
Index: lldb/include/lldb/H

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D128366#3608051 , @labath wrote:

> In D128366#3605484 , @bulbazord 
> wrote:
>
>> In D128366#3603943 , @labath wrote:
>>
>>> Could we move this pruning elsewhere? These values come from the symbol 
>>> file plugins anyway, and they can do a better job at determining which 
>>> language does a particular name belong to.
>>> (OK, they can an also come from the symtab, but there I guess we could 
>>> infer something from the mangling scheme).
>>
>> We're specifically pruning the results from a name lookup, so I'm not sure 
>> where would be a better place to move it.
>
> Well. I guess ideally I would pass whatever information is needed into the 
> lookup functions themselves, so that there is no need for the additional 
> filtering. I don't know why it's done this way but this setup seems 
> inefficient, as we have to generate all these SymbolContext only for them to 
> be (potentially) thrown away...

I see what you mean now. I'll look at this instead, maybe we can get rid of 
`LookupInfo::Prune` altogether. :)




Comment at: lldb/include/lldb/Target/Language.h:320
+  virtual bool NamesAreEquivalentWithContext(
+  const ConstString &user_provided_name,
+  const ConstString &full_name_with_context) const {

labath wrote:
> bulbazord wrote:
> > labath wrote:
> > > I think we're passing ConstStrings by value these days...
> > I see. Is there a reason we do that instead of passing by reference?
> ConstString is just a fancy wrapper around a const char *, so adding a layer 
> of indirection does not really save us anything (in fact, it's the opposite).
Ah, this makes sense. I should have looked at ConstString more closely. Thank 
you! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128366

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

Thank you.

In D50304#3608740 , @jingham wrote:

> In D50304#3592849 , @RamNalamothu 
> wrote:
>
>> In D50304#3586710 , @jingham wrote:
>>
>>> For some reason I'm not getting mail notifications for review changes, 
>>> sorry about that.
>>>
>>> This is certainly better than the original implementation.  Among other 
>>> things, if we find an exact match, we really shouldn't be doing any more 
>>> inexact matches, so after the first exact hit it should have switched exact 
>>> to true.
>>>
>>> But if you had line tables laid out like (this is in increasing order of 
>>> address:
>>>
>>> 10
>>> 20
>>> 30
>>> 10
>>> 16
>>>
>>> and you did "thread until 15", this would find the inexact match at 20, 
>>> switch to an exact match for line 20 and find no other matches.  But the 
>>> gap between 10 & 16 in the line table is maybe an even more plausible place 
>>> to put the line 15 until breakpoint, so maybe we did want to throw a 
>>> breakpoint there as well?
>>
>> @jingham 
>> Nope, with the current patch, we would find the inexact match at 16.
>
> Ah, my bad.  FindLineEntry does end up in FindLineEntryByFileIndexImpl which 
> does the closest match.
>
>>> Regular breakpoint setting has to move inexact breakpoints in much the same 
>>> way.  The code in the BreakpointResolverFileLine::SearchCallback ends up 
>>> calling CompileUnit::ResolveSymbolContext to get the "best" inexact match.  
>>> Maybe it would be better to not do this by hand here in the Until command, 
>>> but reuse the code that we use to move break points more generally?
>>
>> Be it CompileUnit::ResolveSymbolContext or CompileUnit::FindLineEntry, we 
>> end up calling LineTable::FindLineEntryIndexByFileIndex which finds "Exact 
>> match always wins.  Otherwise try to find the closest line > the desired 
>> line." when we pass `exact = false` to it.
>> Given that and we anyway have to extract address list, from what we get out 
>> of CompileUnit::ResolveSymbolContext, for the thread until thread plan to 
>> work with, probably the additional overhead to use 
>> CompileUnit::ResolveSymbolContext here does not make sense. Or am I missing 
>> something?
>
> Finding these addresses happens once in response to a user command, so I 
> highly doubt that the overhead matters one way or the other.  Both end up 
> doing the closest match (I misremembered where that was done) which is the 
> main thing.  ResolveSymbolContext does more work to handle inlined files, but 
> there's no way to do "thread until" from a main CU into inlined source so 
> that doesn't matter.

Make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [lldb] 87a3293 - [lldb] Move Host::SystemLog out of !defined(_WIN32)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-24T11:18:31-07:00
New Revision: 87a32939611ab2972acc7a8e796f8d9e571a6f3a

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

LOG: [lldb] Move Host::SystemLog out of !defined(_WIN32)

The definition of Host::SystemLog was (unintentionally) guarded by
!defined(_WIN32).

Added: 


Modified: 
lldb/source/Host/common/Host.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Host.cpp 
b/lldb/source/Host/common/Host.cpp
index 925ed956f09b..a0834ca15274 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -90,6 +90,10 @@ int __pthread_fchdir(int fildes);
 using namespace lldb;
 using namespace lldb_private;
 
+#if !defined(__APPLE__)
+void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
+#endif
+
 #if !defined(__APPLE__) && !defined(_WIN32)
 static thread_result_t
 MonitorChildProcessThreadFunction(::pid_t pid,
@@ -106,10 +110,6 @@ llvm::Expected 
Host::StartMonitoringChildProcess(
   });
 }
 
-#if !defined(__APPLE__)
-void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
-#endif
-
 #ifndef __linux__
 // Scoped class that will disable thread canceling when it is constructed, and
 // exception safely restore the previous value it when it goes out of scope.



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

[Lldb-commits] [lldb] a57b62d - [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via lldb-commits]

Author: Venkata Ramanaiah Nalamothu
Date: 2022-06-25T00:01:04+05:30
New Revision: a57b62deef37c7f2ec31bca3bf9173a6206bfb9b

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

LOG: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line 
numbers

The requirements for "thread until " are:

a) If any code contributed by  or the nearest subsequent of  is executed before leaving the function, stop
b) If you end up leaving the function w/o triggering (a), then stop

In case of (a), since the  may have multiple entries in the line 
table and the compiler might have scheduled/moved the relevant code across, and 
the lldb does not know the control flow, set breakpoints on all the line table 
entries of best match of  i.e. exact or the nearest subsequent 
line.

Along with the above, currently, CommandObjectThreadUntil is also setting the 
breakpoints on all the subsequent line numbers after the best match and this 
latter part is wrong.

This issue is discussed at 
http://lists.llvm.org/pipermail/lldb-dev/2018-August/013979.html.

In fact, currently `TestStepUntil.py` is not actually testing step until 
scenarios and `test_missing_one` test fails without this patch if tests are 
made to run. Fixed the test as well.

Reviewed By: jingham

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectThread.cpp
lldb/test/API/functionalities/thread/step_until/TestStepUntil.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 037bbafdf8940..9396c36154979 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@ class CommandObjectThreadUntil : public 
CommandObjectParsed {
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
&end_ptr);
 
+// Since not all source lines will contribute code, check if we are
+// setting the breakpoint on the exact line number or the nearest
+// subsequent line number and set breakpoints at all the line table
+// entries of the chosen line number (exact or nearest subsequent).
 for (uint32_t line_number : line_numbers) {
+  LineEntry line_entry;
+  bool exact = false;
   uint32_t start_idx_ptr = index_ptr;
+  start_idx_ptr = sc.comp_unit->FindLineEntry(
+  index_ptr, line_number, nullptr, exact, &line_entry);
+  if (start_idx_ptr != UINT32_MAX)
+line_number = line_entry.line;
+  exact = true;
+  start_idx_ptr = index_ptr;
   while (start_idx_ptr <= end_ptr) {
-LineEntry line_entry;
-const bool exact = false;
 start_idx_ptr = sc.comp_unit->FindLineEntry(
 start_idx_ptr, line_number, nullptr, exact, &line_entry);
 if (start_idx_ptr == UINT32_MAX)

diff  --git a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py 
b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
index 0145b34f31de5..ee25d1343735f 100644
--- a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@ def setUp(self):
 self.greater_than_two = line_number('main.c', 'Greater than or equal 
to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@ def do_until (self, args, until_lines, expected_linenum):
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@ def test_targetting_two_hitting_second (self):
 self.do_until(None, [self.less_than_two, self.greater_than_two], 
self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by 
stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], 
self.back_out_in_main)
 
 



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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa57b62deef37: [lldb] Fix thread step until to not set 
breakpoint(s) on incorrect line numbers (authored by RamNalamothu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/API/functionalities/thread/step_until/TestStepUntil.py


Index: lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
===
--- lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@
 self.greater_than_two = line_number('main.c', 'Greater than or equal 
to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@
 self.do_until(None, [self.less_than_two, self.greater_than_two], 
self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by 
stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], 
self.back_out_in_main)
 
 
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
&end_ptr);
 
+// Since not all source lines will contribute code, check if we are
+// setting the breakpoint on the exact line number or the nearest
+// subsequent line number and set breakpoints at all the line table
+// entries of the chosen line number (exact or nearest subsequent).
 for (uint32_t line_number : line_numbers) {
+  LineEntry line_entry;
+  bool exact = false;
   uint32_t start_idx_ptr = index_ptr;
+  start_idx_ptr = sc.comp_unit->FindLineEntry(
+  index_ptr, line_number, nullptr, exact, &line_entry);
+  if (start_idx_ptr != UINT32_MAX)
+line_number = line_entry.line;
+  exact = true;
+  start_idx_ptr = index_ptr;
   while (start_idx_ptr <= end_ptr) {
-LineEntry line_entry;
-const bool exact = false;
 start_idx_ptr = sc.comp_unit->FindLineEntry(
 start_idx_ptr, line_number, nullptr, exact, &line_entry);
 if (start_idx_ptr == UINT32_MAX)


Index: lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
===
--- lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@
 self.greater_than_two = line_number('main.c', 'Greater than or equal to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@
 self.do_until(None, [self.less_than_two, self.greater_than_two], self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], self.back_out_in_main)
 
 
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
   

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The current way ot traversing the cursor is a bit uncommon and it can't handle 
empty traces, in fact, its invariant is that it shold always point to a valid 
item. This diff simplifies the cursor API and allows it to point to invalid 
items, thus being able to handle empty traces or to know it ran out of data.

- Removed all the granularity functionalities, because we are not actually 
making use of that. We can bring them back when they are actually needed.
- change the looping logic to the following:

  for (; cursor->HasValue(); cursor->Next()) {
 if (cursor->IsError()) {
   .. do something for error
   continue;
 }
 .. do something for instruction
  }



- added a HasValue method that can be used to identify if the cursor ran out of 
data, the trace is empty, or the user tried to move to an invalid position via 
SetId() or Seek()
- made several simplifications to severals parts of the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128543

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -261,33 +261,20 @@
 : m_cursor_up(std::move(cursor_up)), m_options(options),
   m_writer_up(CreateWriter(s, m_options)) {
 
-  if (m_options.id) {
-if (!m_cursor_up->GoToId(*m_options.id)) {
-  m_writer_up->InfoMessage("invalid instruction id");
-  SetNoMoreData();
-}
-  } else if (m_options.forwards) {
+  if (m_options.id)
+m_cursor_up->GoToId(*m_options.id);
+  else if (m_options.forwards)
 m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
-  } else {
+  else
 m_cursor_up->Seek(0, TraceCursor::SeekType::End);
-  }
 
   m_cursor_up->SetForwards(m_options.forwards);
   if (m_options.skip) {
-uint64_t to_skip = *m_options.skip;
-if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
-  TraceCursor::SeekType::Current) < to_skip) {
-  // This happens when the skip value was more than the number of
-  // available instructions.
-  SetNoMoreData();
-}
+m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+  TraceCursor::SeekType::Current);
   }
 }
 
-void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; }
-
-bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
-
 TraceInstructionDumper::InstructionEntry
 TraceInstructionDumper::CreatRawInstructionEntry() {
   InstructionEntry insn;
@@ -375,11 +362,8 @@
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
-  for (size_t i = 0; i < count; i++) {
-if (!HasMoreData()) {
-  m_writer_up->InfoMessage("no more data");
-  break;
-}
+  for (size_t i = 0; i < count && m_cursor_up->HasValue();
+   m_cursor_up->Next(), i++) {
 last_id = m_cursor_up->GetId();
 
 if (m_options.forwards) {
@@ -418,9 +402,8 @@
   // makes sense.
   PrintEvents();
 }
-
-if (!m_cursor_up->Next())
-  SetNoMoreData();
   }
+  if (!m_cursor_up->HasValue())
+m_writer_up->InfoMessage("no more data");
   return last_id;
 }
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
===
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
@@ -154,7 +154,8 @@
   // TODO: Make distinction between errors by storing the error messages.
   // Currently, all errors are treated the same.
   m_instruction_layer_up->AppendInstruction(0);
- 

[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Just FYI, this commit appears to cause 
lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 to fail.  The error I'm seeing is:

Traceback (most recent call last):

  File 
"../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py",
 line 72, in test_target_auto_install_main_executable
self.expect("process launch", substrs=["exited with status = 74"])
  File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in 
expect
self.runCmd(
  File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in 
runCmd
self.assertTrue(self.res.Succeeded(),

AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127702

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

[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added a comment.

@cmtice, are you sure the failure is caused by/related with this patch? This is 
a pure lldb-vscode change which is not used by normal lldb. Also, no mention of 
code in this patch in the error message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127702

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

[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I apologize; I was looking at several commits, and I accidentally added my 
comment to the wrong one. No, this commit is not the one that caused my problem 
(I am very sorry about the confusion).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127702

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

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

[Caroline Tice via Phabricator via lldb-commits]

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

Just FYI, this commit appears to cause 
lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 to fail. The error I'm seeing is:

Traceback (most recent call last):

File 
"../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py",
 line 72, in test_target_auto_install_main_executable

  self.expect("process launch", substrs=["exited with status = 74"])

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in expect

  self.runCmd(

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in runCmd

  self.assertTrue(self.res.Succeeded(),

AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128152

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Add `eLogHandlerDefault = eLogHandlerStream`.


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

https://reviews.llvm.org/D128323

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, eLogHandlerStream, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1126,7 +1126,8 @@
 { eArgTypeCommand, "command", CommandCompletions::eNoCompletion, { nullptr, false }, "An LLDB Command line command element." },
 { eArgTypeColumnNum, "column", CommandCompletions::eNoCompletion, { nullptr, false }, "Column number in a source file." },
 { eArgTypeModuleUUID, "module-uuid", CommandCompletions::eModuleUUIDCompletion, { nullptr, false }, "A module UUID value." },
-{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." }
+{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." },
+{ eArgTypeLogHandler, "log-handler", CommandCompletions::eNoCompletion, { nullptr, false }, "The log handle that will be used to write out log messages." },
 // clang-format on
 };
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1406,11 +1406,27 @@
debugger_id, once);
 }
 
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {
+  switch (log_handler_kind) {
+  case eLogHandlerStream:
+return std::make_shared(fd, should_close, buffer_size);
+  case eLogHandlerCircular:
+return std::make_shared(buffer_size);
+  case eLogHandlerSystem:
+return std::make_shared();
+  case eLogHandlerCallback:
+return {};
+  }
+  return {};
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream) {
-  const bool should_close = true;
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream) {
 
   std::shared_ptr log_handler_sp;
   if (m_callback_handler_sp) {
@@ -1419,8 +1435,9 @@
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close, buffer_size);
+log_handler_sp =
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ /*should_close=*/false, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1440,8 +1457,9 @@
 return false;
   }
 
-  log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close, buffer_size);
+  log_handler_sp =
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   /*should_close=*/true, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -431,8 +431,10 @@
 let Command = "log enable" in {
   def log_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
 Desc<"Set the destination file to log to.">;
+  def log_handler : Option<"log-handler", "h">, Group<1>,
+EnumArg<"LogHandler", "LogHandlerType()">, Desc<"Specify a log handler which determines where log messages are written.">;
  

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128410#3608190 , @labath wrote:

> In D128410#3604927 , @alvinhochun 
> wrote:
>
>> It may be possible to stuff a pointer to an `EXCEPTION_RECORD` into another 
>> `EXCEPTION_RECORD` and use `RtlRaiseException` to generate the exception, 
>> but you'll have to test how it actually works.
>
> That would be pretty cool.

Yeah - I guess it's two separate kinds of testcases; this one would be more of 
a macro-testcase, "does this real-world case work - whichever way lldb happens 
to handle it" (nested exception or not?) while that would be more of a clinical 
unit test for specifically testing nested exceptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128410

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Improve help and error message


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

https://reviews.llvm.org/D128323

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, eLogHandlerStream, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1126,7 +1126,8 @@
 { eArgTypeCommand, "command", CommandCompletions::eNoCompletion, { nullptr, false }, "An LLDB Command line command element." },
 { eArgTypeColumnNum, "column", CommandCompletions::eNoCompletion, { nullptr, false }, "Column number in a source file." },
 { eArgTypeModuleUUID, "module-uuid", CommandCompletions::eModuleUUIDCompletion, { nullptr, false }, "A module UUID value." },
-{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." }
+{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." },
+{ eArgTypeLogHandler, "log-handler", CommandCompletions::eNoCompletion, { nullptr, false }, "The log handle that will be used to write out log messages." },
 // clang-format on
 };
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1406,11 +1406,27 @@
debugger_id, once);
 }
 
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {
+  switch (log_handler_kind) {
+  case eLogHandlerStream:
+return std::make_shared(fd, should_close, buffer_size);
+  case eLogHandlerCircular:
+return std::make_shared(buffer_size);
+  case eLogHandlerSystem:
+return std::make_shared();
+  case eLogHandlerCallback:
+return {};
+  }
+  return {};
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream) {
-  const bool should_close = true;
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream) {
 
   std::shared_ptr log_handler_sp;
   if (m_callback_handler_sp) {
@@ -1419,8 +1435,9 @@
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close, buffer_size);
+log_handler_sp =
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ /*should_close=*/false, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1440,8 +1457,9 @@
 return false;
   }
 
-  log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close, buffer_size);
+  log_handler_sp =
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   /*should_close=*/true, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -431,8 +431,10 @@
 let Command = "log enable" in {
   def log_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
 Desc<"Set the destination file to log to.">;
+  def log_handler : Option<"log-handler", "h">, Group<1>,
+EnumArg<"LogHandler", "LogHandlerType()">, Desc<"Specify a log handler which determines where log messages are written.">;
   def log_buffer

[Lldb-commits] [PATCH] D128557: [lldb] Add a log dump command to dump the circular log buffer

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, mib, clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add a log dump command to dump the circular log buffer. This also adds a test 
for the circular log handler.


https://reviews.llvm.org/D128557

Files:
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/common/Host.cpp
  lldb/source/Utility/Log.cpp
  lldb/test/API/commands/log/basic/TestLogHandlers.py

Index: lldb/test/API/commands/log/basic/TestLogHandlers.py
===
--- /dev/null
+++ lldb/test/API/commands/log/basic/TestLogHandlers.py
@@ -0,0 +1,39 @@
+"""
+Test lldb log handlers.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LogHandlerTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_circular(self):
+self.log_file = self.getBuildArtifact("log-file.txt")
+if (os.path.exists(self.log_file)):
+os.remove(self.log_file)
+
+self.runCmd("log enable -b 5 -h circular lldb commands")
+self.runCmd("bogus", check=False)
+self.runCmd("log dump lldb commands -f {}".format(self.log_file))
+
+with open(self.log_file, 'r') as f:
+log_lines = f.readlines()
+
+self.assertEqual(len(log_lines), 5)
+
+found_command_log_dump = False
+found_command_bogus = False
+
+for line in log_lines:
+if 'Processing command: log dump' in line:
+found_command_log_dump = True
+if 'Processing command: bogus' in line:
+found_command_bogus = True
+
+self.assertTrue(found_command_log_dump)
+self.assertFalse(found_command_bogus)
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -13,6 +13,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator.h"
 
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -34,6 +35,11 @@
 
 using namespace lldb_private;
 
+char LogHandler::ID;
+char StreamLogHandler::ID;
+char CallbackLogHandler::ID;
+char RotatingLogHandler::ID;
+
 llvm::ManagedStatic Log::g_channel_map;
 
 void Log::ForEachCategory(
@@ -106,6 +112,13 @@
   }
 }
 
+void Log::Dump(llvm::raw_ostream &output_stream) {
+  llvm::sys::ScopedReader lock(m_mutex);
+  if (RotatingLogHandler *handler =
+  llvm::dyn_cast(m_handler.get()))
+handler->Dump(output_stream);
+}
+
 const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
@@ -222,6 +235,19 @@
   return true;
 }
 
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::ArrayRef categories,
+ llvm::raw_ostream &output_stream,
+ llvm::raw_ostream &error_stream) {
+  auto iter = g_channel_map->find(channel);
+  if (iter == g_channel_map->end()) {
+error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
+return false;
+  }
+  iter->second.Dump(output_stream);
+  return true;
+}
+
 bool Log::ListChannelCategories(llvm::StringRef channel,
 llvm::raw_ostream &stream) {
   auto ch = g_channel_map->find(channel);
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -632,6 +632,8 @@
   return result;
 }
 
+char SystemLogHandler::ID;
+
 SystemLogHandler::SystemLogHandler() {}
 
 void SystemLogHandler::Emit(llvm::StringRef message) {
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -456,6 +456,11 @@
 Desc<"Prepend the names of files and function that generate the logs.">;
 }
 
+let Command = "log dump" in {
+  def log_dump_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
+Desc<"Set the destination file to dump to.">;
+}
+
 let Command = "reproducer dump" in {
   def reproducer_provider : Option<"provider", "p">, Group<1>,
 EnumArg<"None", "ReproducerProviderType()">,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -56,6 +56,9 @@
 #define LLDB_OPTIONS_log_enable
 #include "CommandOptions.inc"
 
+#define LLDB_OPTIONS_log_dump
+#include "CommandOptions.inc"
+
 /// Common completion logic for log enable/disable.
 static void CompleteEnableDisable(Complet

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Commands/CommandObjectLog.cpp:167
   buffer_size.Clear();
+  handler = eLogHandlerStream;
   log_options = 0;

labath wrote:
> mib wrote:
> > clayborg wrote:
> > > Do we want to define a "eLogHandlerDefault" which points to 
> > > "eLogHandlerStream"?
> > I was hoping we could use a type alias for this, that marks 
> > `eLogHandlerStream` as `eLogHandlerDefault` but I couldn't find of a nice 
> > C++ way to do it :/ 
> Were you maybe looking for something like:
> ```
> enum {
>  foo_1,
>  foo_2,
>  ...,
>   foo_default = foo_47
> };
> ```
That could do :) ! I was thinking of doing :

```
enum {
foo_1,
foo_2,
...
};

using foo_default = foo_1;
```

But I don't think this works.




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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D128557: [lldb] Add a log dump command to dump the circular log buffer

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM!


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

https://reviews.llvm.org/D128557

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

[Lldb-commits] [PATCH] D128557: [lldb] Add a log dump command to dump the circular log buffer

[Greg Clayton via Phabricator via lldb-commits]

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

So we either need to track categories for each buffered log message in the 
circular buffer so that if the user specifies a category within a channel to 
dump with "log dump lldb " we can filter out the messages we dump if 
they are not in the one or more categories that were supplied, or remove 
categories from the "log dump" command and only specify the channel. Easy to 
fix the code to work either way.

The idea is if the user does:

  (lldb) log enable -b 5 -h circular lldb command state process
  (lldb) ...
  (lldb log dump lldb process

I would expect only "process" category messages to be shown. The current code 
allows the user to specify categories, but they don't do anything. So we should 
either obey the categories correctly by changing "LogHandler::Emit(StringRef 
message)" to take an extra category mask like "LogHandler::Emit(StringRef 
message, uint64_t category)" so that we can cache a vector of 
"std::pair" so we can emit only the requested categories 
when using "log dump", or just remove the categories from the "log dump" 
command.




Comment at: lldb/source/Commands/CommandObjectLog.cpp:359
+CommandArgumentData channel_arg;
+CommandArgumentData category_arg;
+

If we aren't going to correctly track what category that each log channel comes 
from, then specifying the categories doesn't do anything and the categories 
shouldn't be part of this command unless you want to fix that. We could easily 
do this if we modify LogHandler::Emit(...) to take a category integer and then 
our buffer would need to store category + message in an array.



Comment at: lldb/source/Utility/Log.cpp:239
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::ArrayRef categories,
+ llvm::raw_ostream &output_stream,

We don't need categories here. We have no way to separate each log message 
since we don't store this, and the user shouldn't be required to know the 
categories right?



Comment at: lldb/test/API/commands/log/basic/TestLogHandlers.py:20
+
+self.runCmd("log enable -b 5 -h circular lldb commands")
+self.runCmd("bogus", check=False)

Do we want to test that non circular buffers can't be dumped too to make sure 
nothing goes wrong or that we get a good error message stating that this log 
channel isn't circular?



Comment at: lldb/test/API/commands/log/basic/TestLogHandlers.py:22
+self.runCmd("bogus", check=False)
+self.runCmd("log dump lldb commands -f {}".format(self.log_file))
+

It seems wrong to specify the categories in the "log dump" command since we 
won't do the right thing. If you can specified categories I would expect this 
to work:
```
(lldb) log enable -b 5 -h circular lldb command state process
(lldb) ...
(lldb) log dump lldb process
```
And that only process log lines would come out. Since we don't store this 
information, we shouldn't have to specify the categories at all, so the "log 
dump" command should just take the channel:
```
(lldb) log dump lldb
```



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

https://reviews.llvm.org/D128557

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

[Lldb-commits] [PATCH] D128504: debugserver: spawn process in its own process group

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Seems to make sense to me, but I will let Jason Molenda give the final OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128504

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

[Lldb-commits] [PATCH] D128453: Automate checking for "command that takes no arguments" being passed arguments in CommandObjectParsed

[Stephen Hines via Phabricator via lldb-commits]

srhines added a comment.

In D128453#3607129 , @labath wrote:

> In D128453#3606296 , @JDevlieghere 
> wrote:
>
>> This is great, it both guarantees consistently and enforces command objects 
>> registering their arguments. LGTM.
>>
>> For the RenderScript plugin, I remember this at an LLVM social when @labath 
>> was in town. At the time, we were very close to being able to get rid of it, 
>> which is now several years ago. Pavel, do you remember who we spoke to and 
>> if we've reached the point where this can go away?
>
> I don't know what's the state of renderscript, but I think @srhines is the 
> person you have in mind.

The RS-related changes here look fine for now. I'm trying to get confirmation 
on whether we can remove it entirely at this point (but you shouldn't do that 
in this patch anyways).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128453

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

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[Jakob Johnson via Phabricator via lldb-commits]

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

will take a complete look over the weekend, but wanted to point out the 
conflict with @persona0220's diff asap




Comment at: lldb/include/lldb/Target/TraceCursor.h:93-98
-  /// Set the granularity to use in the \a TraceCursor::Next() method.
-  void SetGranularity(lldb::TraceInstructionControlFlowType granularity);
-
-  /// Set the "ignore errors" flag to use in the \a TraceCursor::Next() method.
-  void SetIgnoreErrors(bool ignore_errors);
-

@persona0220 did this in https://reviews.llvm.org/D128477 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

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

[Lldb-commits] [PATCH] D128557: [lldb] Add a log dump command to dump the circular log buffer

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Thanks for pointing that out. I blindly copied the categories logic from 
`Log::Disable` which uses it when computing the flags. I've omitted it for now 
because I think it would be weird to set the circular buffer size to 5 and then 
have 0 messages printed for the process category because 5 log messages from 
the commands category pushed them out. One solution would be to keep a circular 
buffer per category. I'll think about it and if I come up with a good solution 
I'll put up another patch.


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

https://reviews.llvm.org/D128557

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

[Lldb-commits] [PATCH] D128557: [lldb] Add a log dump command to dump the circular log buffer

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 439921.

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

https://reviews.llvm.org/D128557

Files:
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/common/Host.cpp
  lldb/source/Utility/Log.cpp
  lldb/test/API/commands/log/basic/TestLogHandlers.py

Index: lldb/test/API/commands/log/basic/TestLogHandlers.py
===
--- /dev/null
+++ lldb/test/API/commands/log/basic/TestLogHandlers.py
@@ -0,0 +1,47 @@
+"""
+Test lldb log handlers.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LogHandlerTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_circular(self):
+self.log_file = self.getBuildArtifact("log-file.txt")
+if (os.path.exists(self.log_file)):
+os.remove(self.log_file)
+
+self.runCmd("log enable -b 5 -h circular lldb commands")
+self.runCmd("bogus", check=False)
+self.runCmd("log dump lldb -f {}".format(self.log_file))
+
+with open(self.log_file, 'r') as f:
+log_lines = f.readlines()
+
+self.assertEqual(len(log_lines), 5)
+
+found_command_log_dump = False
+found_command_bogus = False
+
+for line in log_lines:
+if 'Processing command: log dump' in line:
+found_command_log_dump = True
+if 'Processing command: bogus' in line:
+found_command_bogus = True
+
+self.assertTrue(found_command_log_dump)
+self.assertFalse(found_command_bogus)
+
+def test_circular_no_buffer_size(self):
+self.expect(
+"log enable -h circular lldb commands",
+error=True,
+substrs=[
+'the circular buffer handler requires a non-zero buffer size'
+])
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -13,6 +13,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator.h"
 
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -34,6 +35,11 @@
 
 using namespace lldb_private;
 
+char LogHandler::ID;
+char StreamLogHandler::ID;
+char CallbackLogHandler::ID;
+char RotatingLogHandler::ID;
+
 llvm::ManagedStatic Log::g_channel_map;
 
 void Log::ForEachCategory(
@@ -106,6 +112,13 @@
   }
 }
 
+void Log::Dump(llvm::raw_ostream &output_stream) {
+  llvm::sys::ScopedReader lock(m_mutex);
+  if (RotatingLogHandler *handler =
+  llvm::dyn_cast_or_null(m_handler.get()))
+handler->Dump(output_stream);
+}
+
 const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
@@ -222,6 +235,18 @@
   return true;
 }
 
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::raw_ostream &output_stream,
+ llvm::raw_ostream &error_stream) {
+  auto iter = g_channel_map->find(channel);
+  if (iter == g_channel_map->end()) {
+error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
+return false;
+  }
+  iter->second.Dump(output_stream);
+  return true;
+}
+
 bool Log::ListChannelCategories(llvm::StringRef channel,
 llvm::raw_ostream &stream) {
   auto ch = g_channel_map->find(channel);
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -632,6 +632,8 @@
   return result;
 }
 
+char SystemLogHandler::ID;
+
 SystemLogHandler::SystemLogHandler() {}
 
 void SystemLogHandler::Emit(llvm::StringRef message) {
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -456,6 +456,11 @@
 Desc<"Prepend the names of files and function that generate the logs.">;
 }
 
+let Command = "log dump" in {
+  def log_dump_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
+Desc<"Set the destination file to dump to.">;
+}
+
 let Command = "reproducer dump" in {
   def reproducer_provider : Option<"provider", "p">, Group<1>,
 EnumArg<"None", "ReproducerProviderType()">,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -56,6 +56,9 @@
 #define LLDB_OPTIONS_log_enable
 #include "CommandOptions.inc"
 
+#define LLDB_OPTIONS_log_dump
+#include "CommandOptions.inc"
+
 /// Common completion logic for log enable

[Lldb-commits] [lldb] be265d2 - [lldb] Add support for specifying a log handler

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-24T18:24:00-07:00
New Revision: be265d25ca5e300a3af45a5f85fc6e4b107148e5

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

LOG: [lldb] Add support for specifying a log handler

This patch adds a new flag to `log enable`, allowing the user to specify
a custom log handler. In addition to the default (stream) handler, this
allows using the circular log handler (which logs to a fixed size,
in-memory circular buffer) as well as the system log handler (which logs
to the operating system log).

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/lldb-enumerations.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/source/API/SBDebugger.cpp
lldb/source/Commands/CommandObjectLog.cpp
lldb/source/Commands/Options.td
lldb/source/Core/Debugger.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/tools/lldb-test/lldb-test.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index b5e6c30f5f77a..031c9a9674d7e 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -245,7 +245,8 @@ class Debugger : public 
std::enable_shared_from_this,
   bool EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream);
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream);
 
   void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
 

diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 41dbeebad9b05..74a82f6216c7f 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -602,6 +602,7 @@ enum CommandArgumentType {
   eArgTypeColumnNum,
   eArgTypeModuleUUID,
   eArgTypeSaveCoreStyle,
+  eArgTypeLogHandler,
   eArgTypeLastArg // Always keep this entry as the last entry in this
   // enumeration!!
 };

diff  --git a/lldb/include/lldb/lldb-private-enumerations.h 
b/lldb/include/lldb/lldb-private-enumerations.h
index 9bbb889359b12..2d13e6ef65e1f 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -222,6 +222,14 @@ enum StatisticKind {
   StatisticMax = 4
 };
 
+// Enumeration that can be used to specify a log handler.
+enum LogHandlerKind {
+  eLogHandlerStream,
+  eLogHandlerCallback,
+  eLogHandlerCircular,
+  eLogHandlerSystem,
+  eLogHandlerDefault = eLogHandlerStream,
+};
 
 inline std::string GetStatDescription(lldb_private::StatisticKind K) {
switch (K) {

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 8aa2c74d9e378..0de934b2a9bd9 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1621,7 +1621,8 @@ bool SBDebugger::EnableLog(const char *channel, const 
char **categories) {
 std::string error;
 llvm::raw_string_ostream error_stream(error);
 return m_opaque_sp->EnableLog(channel, GetCategoryArray(categories), "",
-  log_options, /*buffer_size=*/0, 
error_stream);
+  log_options, /*buffer_size=*/0,
+  eLogHandlerStream, error_stream);
   } else
 return false;
 }

diff  --git a/lldb/source/Commands/CommandObjectLog.cpp 
b/lldb/source/Commands/CommandObjectLog.cpp
index 91277e33cf360..349af26691de9 100644
--- a/lldb/source/Commands/CommandObjectLog.cpp
+++ b/lldb/source/Commands/CommandObjectLog.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValueEnumeration.h"
 #include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/Args.h"
@@ -22,6 +23,36 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static constexpr OptionEnumValueElement g_log_handler_type[] = {
+{
+eLogHandlerDefault,
+"default",
+"Use the default (stream) log handler",
+},
+{
+eLogHandlerStream,
+"stream",
+"Write log messages to the debugger output stream or to a file if one "
+"is specified. A buffer size (in bytes) can be specified with -b. If "
+"no buffer size is specified the output is unbuffered.",
+},
+{
+eLogHandlerCircular,
+"circular",
+"Write log messages to a fixed size circular buffer. A buffer size "
+

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe265d25ca5e: [lldb] Add support for specifying a log 
handler (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128323

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, eLogHandlerStream, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1126,7 +1126,8 @@
 { eArgTypeCommand, "command", CommandCompletions::eNoCompletion, { nullptr, false }, "An LLDB Command line command element." },
 { eArgTypeColumnNum, "column", CommandCompletions::eNoCompletion, { nullptr, false }, "Column number in a source file." },
 { eArgTypeModuleUUID, "module-uuid", CommandCompletions::eModuleUUIDCompletion, { nullptr, false }, "A module UUID value." },
-{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." }
+{ eArgTypeSaveCoreStyle, "corefile-style", CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile that lldb will try to create, dependant on this target's capabilities." },
+{ eArgTypeLogHandler, "log-handler", CommandCompletions::eNoCompletion, { nullptr, false }, "The log handle that will be used to write out log messages." },
 // clang-format on
 };
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1406,11 +1406,27 @@
debugger_id, once);
 }
 
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {
+  switch (log_handler_kind) {
+  case eLogHandlerStream:
+return std::make_shared(fd, should_close, buffer_size);
+  case eLogHandlerCircular:
+return std::make_shared(buffer_size);
+  case eLogHandlerSystem:
+return std::make_shared();
+  case eLogHandlerCallback:
+return {};
+  }
+  return {};
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream) {
-  const bool should_close = true;
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream) {
 
   std::shared_ptr log_handler_sp;
   if (m_callback_handler_sp) {
@@ -1419,8 +1435,9 @@
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close, buffer_size);
+log_handler_sp =
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ /*should_close=*/false, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1440,8 +1457,9 @@
 return false;
   }
 
-  log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close, buffer_size);
+  log_handler_sp =
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   /*should_close=*/true, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -431,8 +431,10 @@
 let Command = "log enable" in {
   def log_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
 Desc<"Set the destination file to log to.">;
+  def lo

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

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

mgorny added a comment.

Thanks for the notice. I was pretty sure it failed to me prior to my changes 
but it is possible that I read the output wrong, so I'm going to check it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128152

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

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

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

mgorny added a comment.

I'm sorry, it is indeed my fault. I'm going to try to fix it shortly, and if I 
don't manage to figure it out, I'm going to revert this and the commits 
depending on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128152

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