[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add partial read/write tests for x87 regs

[Pavel Labath via Phabricator via lldb-commits]

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

This is great. Thanks.




Comment at: lldb/test/Shell/Register/x86-fp-read.test:7
+register read --all
+# CHECK-DAG: fctrl = 0x037e
+# CHECK-DAG: fstat = 0x88c1

Maybe add a `# CHECK: Process {{.*}} stopped` line here. That will restrict the 
following CHECK-DAGs to match after it, which will make FileCheck output more 
useful. Without it, FileCheck's "Searching from here" snippet will start at the 
very beginning of the lldb output, which will not be very helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D87868#2304680 , @clayborg wrote:

> That is possible, though how do we figure out the syscall enumeration for the 
> mmap syscall reliably for a given system? And if we are remote debugging? 
> Seems like we are signing up for architecture specific syscalls on every 
> different architecture. Not sure I would go that route, but I am open to 
> seeing a solution before passing judgement.

Well.. we already need to "know" the right target-specific values for the 
various PROT_ and MAP_ flags, so I don't think including the syscall number 
(and the syscall opcode) would not be too much of a stretch.

>> Note that this would not need to be implemented in the lldb client. This 
>> sort of thing would be natural to implement in lldb server in response to 
>> the `_M` packet. There it would be easy to encode the abi details needed to 
>> issue a syscall. The client already prefers this packet, and the existing 
>> code could remain as a fallback for platforms not implementing it.
>
> There is no code currently that does any run control down inside of 
> lldb-server and I would like to keep it that way if possible. debugserver can 
> do it only because we have the task port to the program we are debugging and 
> we can call a function in the debugserver process that does the memory 
> allocation in the debug process. Having this done in lldb-server would 
> require lldb-server to perform run control by changing register values, 
> running the syscall, stopping at a breakpoint to after the syscall has run, 
> removing that breakpoint only if it didn't exist already. Seems like a lot of 
> dangerous flow control that we won't be able to see if anything goes wrong. 
> Right now if we are doing it all through the GDB remote protocol, we can see 
> exactly how things go wrong in the packet log, but now it would be a mystery 
> if things go wrong.

I do share that sentiment, and if the setup needed for this would be anything 
like the mmap dance, I wouldn't even try it. The only reason I brought this up 
is because I expect the syscall routine to be much simpler (like, maybe simpler 
than the instruction emulation routine that we do on arm). There's no need to 
set breakpoints, as we're just PTRACE_SINGLESTEPing over a single instruction 
(unless we're on arm of course). So the implementation would be something like:

- save all registers
- find first executable page
- write "int 0x80" to the first two bytes
- setup registers for a syscall (including pointing pc to the int 0x80 
instruction)
- PTRACE_SINGLESTEP+waitpid
- fetch result from %rax
- restore bytes overwritten by the int 0x80
- restore all registers

I don't think this would be too complicated, though it would obviously have to 
be done with a steady hand. In fact, one of the original use cases for the 
linux ptrace syscall was the ability for the tracer to track/replace/emulate 
syscalls done by the traced process. Unfortunately, the intended use was a bit 
different (things like qemu and user-mode linux) and it assumes the tracer 
wants to modify a syscall that the tracee already wanted to make (and not 
inject a new one "out of the blue"). This means we cannot use the existing 
support for that. That said, I think I've managed to convince the NetBSD folks 
to include out-of-blue syscall support in their ptrace implementation. Maybe I 
should try the same for linux... :P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87868

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

[Lldb-commits] [PATCH] D88266: Check that the "StructuredData Plugin weak pointer" is good before trying to turn it into a shared pointer

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Sounds like a job for a clang-tidy check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88266

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

mgorny updated this revision to Diff 295500.
mgorny retitled this revision from "[lldb] [test/Register] Add partial 
read/write tests for x87 regs" to "[lldb] [test/Register] Add read/write tests 
for x87 regs".
mgorny edited the summary of this revision.
mgorny added a comment.

Done as requested + improved the tests:

- switch to `FDIV` on memory operand, to be able to test FDP
- added disassembly-based test for FIP and print-based test for FDP (assuming 
Linux behavior for fiseg/fioff/foseg/fooff, this certainly breaks NetBSD and 
might break some more platforms)
- figured out how to make write test work while restoring exception state
- added FXSAVE-based check whether writing other registers works (this also 
includes potential future support for MXCSR)


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

https://reviews.llvm.org/D88583

Files:
  lldb/test/Shell/Register/Inputs/x86-fp-read.cpp
  lldb/test/Shell/Register/Inputs/x86-fp-write.cpp
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test

Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -0,0 +1,47 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
+register write fop 0x0033
+# the exact addresses do not matter, we want just to verify FXSAVE
+register write fiseg 0x01234567
+register write fioff 0x89abcdef
+register write foseg 0xfedcba98
+register write fooff 0x76543210
+
+register write st0 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}"
+register write st1 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}"
+register write st2 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+register write st3 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}"
+register write st4 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}"
+register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}"
+register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}"
+register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+
+process continue
+
+# CHECK: process continue
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# CHECK-DAG: ftag = 0xa961
+# CHECK-DAG: fop = 0x0033
+# TODO: bug?
+# CHECK-DAG: fip = 0x89abcdef
+# CHECK-DAG: fdp = 0x76543210
+
+# CHECK-DAG: st0 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40 }
+# CHECK-DAG: st1 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00 }
+# CHECK-DAG: st2 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+# CHECK-DAG: st3 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 }
+# CHECK-DAG: st4 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f }
+# CHECK-DAG: st5 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff }
+# CHECK-DAG: st6 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff }
+# CHECK-DAG: st7 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -0,0 +1,31 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/x86-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{.*}} stopped
+
+register read --all
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted; it should be 0xa963 once fixed
+# CHECK-DAG: ftag = 0x00fe
+# CHECK-DAG: fop = 0x0033
+
+# CHECK-DAG: st0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
+# CHECK-DAG: st1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}
+# CHECK-DAG: st2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: st3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}
+# CHECK-DAG: st4 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}
+# CHECK-DAG: st5 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}
+# CHECK-DAG: st6 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}
+# CHECK-DAG: st7 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+# TODO: we probably should not split it like this
+disassemble -s '$fiseg*0x1 + $fioff' -c 1
+# CHECK: fdivs (%{{[er]}}bx)
+print *(uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t) $1 = 0
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-fp-write.cpp
==

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

mgorny added a comment.

I'd use some help improving the assembly, as explained below.




Comment at: lldb/test/Shell/Register/Inputs/x86-fp-write.cpp:23
+"rex.w = 1\n\t"
+"fxsave %2\n\t"
+"fnstenv %1\n\t"

Now, here's the problem. This works just fine when built with clang but 
segfaults when built with gcc (apparently tries to access `0x0`).


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

mgorny added inline comments.



Comment at: lldb/test/Shell/Register/Inputs/x86-fp-write.cpp:23
+"rex.w = 1\n\t"
+"fxsave %2\n\t"
+"fnstenv %1\n\t"

mgorny wrote:
> Now, here's the problem. This works just fine when built with clang but 
> segfaults when built with gcc (apparently tries to access `0x0`).
Turned out to be an alignment issue. Will update the patch shortly.


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/Shell/Register/x86-fp-read.test:27-28
+# CHECK: fdivs (%{{[er]}}bx)
+print *(uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t) $1 = 0
+

zero is pretty common value. It would be better to check that the address 
actually points to the address of the `zero` variable. I think something like 
this ought to do it:
```
print &zero
# CHECK: (uint32_t *) $0 = [[ZERO:0x[0-9a-f]*]]
print (uint32_t*)($foseg * 0x1 + $fooff)
# CHECK: (uint32_t *) $1 = [[ZERO]]
```
Something similar can be done with the disassemble command too. That way we'll 
see the actual register value in case it turns out to be wrong.


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

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

Fixed segfault when built with GCC and fixed my poor attempt to run FXSAVE with 
REX.W prefix. Now it correctly gets 64-bit FIP and FDP, though it's unclear 
whether the apparent truncation is a bug in LLDB writing it or some other kind 
of limitation.


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

https://reviews.llvm.org/D88583

Files:
  lldb/test/Shell/Register/Inputs/x86-fp-read.cpp
  lldb/test/Shell/Register/Inputs/x86-fp-write.cpp
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test

Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -0,0 +1,45 @@
+# REQUIRES: native && target-x86
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
+register write fop 0x0033
+# the exact addresses do not matter, we want just to verify FXSAVE
+# note: segment registers are not supported on all CPUs
+register write fioff 0x89abcdef
+register write fooff 0x76543210
+
+register write st0 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}"
+register write st1 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}"
+register write st2 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+register write st3 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}"
+register write st4 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}"
+register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}"
+register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}"
+register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+
+process continue
+
+# CHECK: process continue
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# CHECK-DAG: ftag = 0xa961
+# CHECK-DAG: fop = 0x0033
+# CHECK-DAG: fip = 0x89abcdef
+# CHECK-DAG: fdp = 0x76543210
+
+# CHECK-DAG: st0 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40 }
+# CHECK-DAG: st1 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00 }
+# CHECK-DAG: st2 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+# CHECK-DAG: st3 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 }
+# CHECK-DAG: st4 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f }
+# CHECK-DAG: st5 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff }
+# CHECK-DAG: st6 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff }
+# CHECK-DAG: st7 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -0,0 +1,31 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/x86-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{.*}} stopped
+
+register read --all
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted; it should be 0xa963 once fixed
+# CHECK-DAG: ftag = 0x00fe
+# CHECK-DAG: fop = 0x0033
+
+# CHECK-DAG: st0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
+# CHECK-DAG: st1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}
+# CHECK-DAG: st2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: st3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}
+# CHECK-DAG: st4 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}
+# CHECK-DAG: st5 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}
+# CHECK-DAG: st6 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}
+# CHECK-DAG: st7 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+# TODO: we probably should not split it like this
+disassemble -s '$fiseg*0x1 + $fioff' -c 1
+# CHECK: fdivs (%{{[er]}}bx)
+print *(uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t) $1 = 0
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -0,0 +1,47 @@
+# REQUIRES: native && target-x86_64
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write 

Re: [Lldb-commits] [lldb] 1b1d981 - Revert "Revert "Add the ability to write target stop-hooks using the ScriptInterpreter.""

[Pavel Labath via lldb-commits]

On 30/09/2020 23:21, Jim Ingham wrote:
> The test doesn’t seem to be flakey in the “run it a bunch of times and
> it will eventually fail” type flakey.  I ran the test 200 times on my
> machine and didn’t get a failure.

Actually, it seems like exactly the typical kind of flaky test to me --
it mostly works when run on its own, but starts failing as soon as the
system comes under load.

It didn't fail for me either for over 100 iterations. However, as soon
as I cranked up the cpu load (compiling llvm is good at that), it failed
almost immediately.

It also doesn't seem to be related to the way the stop hook resumes the
process.

is one example where the auto_continue version of the test fails, and I
have seen both tests fail on my machine.

I have some traces of failing and successful runs of the test (will send
them to you in a private email). I didn't dive too deeply, but the
problem does not seem to be related to python stop hooks. It looks more
like a general stop hook bug, which we've had problems with in the past.

The problems seems to be that the process.Continue() on the main thread
returns early, and so the subsequent checks (for the topmost frame etc.)
execute concurrently with the "step out" action. In the "Failure" file
I'll send you you can see that (line 9222) SBFrame::GetFunctionName is
called before the inferior process stops in the main function (the
processing of that happens immediately after that line, on the
"intern-state" thread).

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

[Lldb-commits] [lldb] b272250 - [lldb] Skip the flakey part of TestStopHookScripted on Linux

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-10-01T14:24:38+02:00
New Revision: b272250221595b14c32db6721a0ae4e5f17ea4d2

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

LOG: [lldb] Skip the flakey part of TestStopHookScripted on Linux

This test seems to randomly fail on Linux machines. It's only one part of the
test failing randomly, so let's just skip it instead of reverting the whole
patch (again).

Added: 


Modified: 
lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py

Removed: 




diff  --git a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py 
b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
index e650778fe8e3..a17f7131d20f 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
@@ -71,6 +71,8 @@ def test_stop_hooks_scripted_return_false(self):
 """Test that the returning False from a stop hook works"""
 self.do_test_auto_continue(True)
 
+# Test is flakey on Linux.
+@skipIfLinux
 def do_test_auto_continue(self, return_true):
 """Test that auto-continue works."""
 # We set auto-continue to 1 but the stop hook only applies to 
step_out_of_me,



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

Re: [Lldb-commits] [lldb] 1b1d981 - Revert "Revert "Add the ability to write target stop-hooks using the ScriptInterpreter.""

[Raphael “Teemperor” Isemann via lldb-commits]

+1, I have two machines with very similar setup where only the one that is 
under heavy load sees the test failures.

- Raphael

> On 1 Oct 2020, at 14:24, Pavel Labath  wrote:
> 
> On 30/09/2020 23:21, Jim Ingham wrote:
>> The test doesn’t seem to be flakey in the “run it a bunch of times and
>> it will eventually fail” type flakey.  I ran the test 200 times on my
>> machine and didn’t get a failure.
> 
> Actually, it seems like exactly the typical kind of flaky test to me --
> it mostly works when run on its own, but starts failing as soon as the
> system comes under load.
> 
> It didn't fail for me either for over 100 iterations. However, as soon
> as I cranked up the cpu load (compiling llvm is good at that), it failed
> almost immediately.
> 
> It also doesn't seem to be related to the way the stop hook resumes the
> process.
> 
> is one example where the auto_continue version of the test fails, and I
> have seen both tests fail on my machine.
> 
> I have some traces of failing and successful runs of the test (will send
> them to you in a private email). I didn't dive too deeply, but the
> problem does not seem to be related to python stop hooks. It looks more
> like a general stop hook bug, which we've had problems with in the past.
> 
> The problems seems to be that the process.Continue() on the main thread
> returns early, and so the subsequent checks (for the topmost frame etc.)
> execute concurrently with the "step out" action. In the "Failure" file
> I'll send you you can see that (line 9222) SBFrame::GetFunctionName is
> called before the inferior process stops in the main function (the
> processing of that happens immediately after that line, on the
> "intern-state" thread).
> 
> pl

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

mgorny added inline comments.



Comment at: lldb/test/Shell/Register/x86-fp-read.test:27-28
+# CHECK: fdivs (%{{[er]}}bx)
+print *(uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t) $1 = 0
+

labath wrote:
> zero is pretty common value. It would be better to check that the address 
> actually points to the address of the `zero` variable. I think something like 
> this ought to do it:
> ```
> print &zero
> # CHECK: (uint32_t *) $0 = [[ZERO:0x[0-9a-f]*]]
> print (uint32_t*)($foseg * 0x1 + $fooff)
> # CHECK: (uint32_t *) $1 = [[ZERO]]
> ```
> Something similar can be done with the disassemble command too. That way 
> we'll see the actual register value in case it turns out to be wrong.
Cool idea, thanks!

As for FIP, I suppose that would require getting address corresponding to the 
FDIV instruction somehow. I suppose we can rely on constant-length encoding on 
`int3` and `fdivs ...`, so I guess `$rip-3` might work.


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D88583#2305815 , @mgorny wrote:

> Fixed segfault when built with GCC and fixed my poor attempt to run FXSAVE 
> with REX.W prefix. Now it correctly gets 64-bit FIP and FDP, though it's 
> unclear whether the apparent truncation is a bug in LLDB writing it or some 
> other kind of limitation.

I think that could be an architectural cpu limitation. IIUC, even though the 
addresses are nominally 64-bit, not all addresses within that space are 
actually usable -- only the topmost and bottommost XX GB (TB) of the address 
space are. It could be that the cpu does not actually store all of the bits of 
these registers, but rather just sign-extends the most significant usable bit.


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [lldb] cccb7cf - [lldb] Add missing import for LLDB test decorators to TestStopHookScripted

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-10-01T14:33:13+02:00
New Revision: cccb7cf1a52f38182f56d947bd609027726c778b

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

LOG: [lldb] Add missing import for LLDB test decorators to TestStopHookScripted

This test wasn't using decorators before and was missing the import, so my
previous commit broke the test.

Added: 


Modified: 
lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py

Removed: 




diff  --git a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py 
b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
index a17f7131d20f..014890e0d973 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
@@ -7,7 +7,7 @@
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
-
+from lldbsuite.test.decorators import *
 
 class TestStopHooks(TestBase):
 



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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/Shell/Register/x86-fp-read.test:27-28
+# CHECK: fdivs (%{{[er]}}bx)
+print *(uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t) $1 = 0
+

mgorny wrote:
> labath wrote:
> > zero is pretty common value. It would be better to check that the address 
> > actually points to the address of the `zero` variable. I think something 
> > like this ought to do it:
> > ```
> > print &zero
> > # CHECK: (uint32_t *) $0 = [[ZERO:0x[0-9a-f]*]]
> > print (uint32_t*)($foseg * 0x1 + $fooff)
> > # CHECK: (uint32_t *) $1 = [[ZERO]]
> > ```
> > Something similar can be done with the disassemble command too. That way 
> > we'll see the actual register value in case it turns out to be wrong.
> Cool idea, thanks!
> 
> As for FIP, I suppose that would require getting address corresponding to the 
> FDIV instruction somehow. I suppose we can rely on constant-length encoding 
> on `int3` and `fdivs ...`, so I guess `$rip-3` might work.
I was thinking you could disassemble the `main` function and capture the 
address before the `fdivs` instruction. But yes, a fixed offset from `$rip` 
should work just fine...


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

https://reviews.llvm.org/D88583

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

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

Update fp-read tests to verify FIP/FDP addresses, as suggested by Pavel.


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

https://reviews.llvm.org/D88583

Files:
  lldb/test/Shell/Register/Inputs/x86-fp-read.cpp
  lldb/test/Shell/Register/Inputs/x86-fp-write.cpp
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test

Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -0,0 +1,45 @@
+# REQUIRES: native && target-x86
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
+register write fop 0x0033
+# the exact addresses do not matter, we want just to verify FXSAVE
+# note: segment registers are not supported on all CPUs
+register write fioff 0x89abcdef
+register write fooff 0x76543210
+
+register write st0 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}"
+register write st1 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}"
+register write st2 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+register write st3 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}"
+register write st4 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}"
+register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}"
+register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}"
+register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+
+process continue
+
+# CHECK: process continue
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# CHECK-DAG: ftag = 0xa961
+# CHECK-DAG: fop = 0x0033
+# CHECK-DAG: fip = 0x89abcdef
+# CHECK-DAG: fdp = 0x76543210
+
+# CHECK-DAG: st0 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40 }
+# CHECK-DAG: st1 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00 }
+# CHECK-DAG: st2 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+# CHECK-DAG: st3 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 }
+# CHECK-DAG: st4 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f }
+# CHECK-DAG: st5 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff }
+# CHECK-DAG: st6 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff }
+# CHECK-DAG: st7 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -0,0 +1,36 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/x86-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{.*}} stopped
+
+register read --all
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted; it should be 0xa963 once fixed
+# CHECK-DAG: ftag = 0x00fe
+# CHECK-DAG: fop = 0x0033
+
+# CHECK-DAG: st0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
+# CHECK-DAG: st1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}
+# CHECK-DAG: st2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: st3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}
+# CHECK-DAG: st4 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}
+# CHECK-DAG: st5 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}
+# CHECK-DAG: st6 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}
+# CHECK-DAG: st7 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+# fdiv (%rbx) gets encoded into 2 bytes, int3 into 1 byte
+print (void*)($pc-3)
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+# TODO: we probably should not split it like this
+print (void*)($fiseg*0x1 + $fioff)
+# CHECK: (void *) $1 = [[FDIV]]
+print &zero
+# CHECK: (uint32_t *) $2 = [[ZERO:0x[0-9a-f]*]]
+print (uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t *) $3 = [[ZERO]]
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -0,0 +1,47 @@
+# REQUIRES: native && target-x86_64
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+reg

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

cool


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

https://reviews.llvm.org/D88583

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

Re: [Lldb-commits] [lldb] 92e1ebe - [trace] Fix destructor declaration

[Pavel Labath via lldb-commits]

On 30/09/2020 20:25, Walter Erquinigo wrote:
> But what about the case Vedant mentioned?
> 
>>Otherwise, when a std::shared_ptr is destroyed, the destructor for 
>> the derived TraceIntelPT instance won't run.
> 
> Or is C++ smart enough to pick the destructor from TraceIntelPT class in this 
> case? 

Well, yes. Once a base class has a virtual distructor, the destructors
of all derived classes will be virtual (no matter how they are
declared). The virtualness guarantees that the destructor of the actual
(runtime) class type gets invoked.

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

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

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

Use safer values for `fiseg` and `foseg`, to avoid truncation.


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

https://reviews.llvm.org/D88583

Files:
  lldb/test/Shell/Register/Inputs/x86-fp-read.cpp
  lldb/test/Shell/Register/Inputs/x86-fp-write.cpp
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test

Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -0,0 +1,45 @@
+# REQUIRES: native && target-x86
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
+register write fop 0x0033
+# the exact addresses do not matter, we want just to verify FXSAVE
+# note: segment registers are not supported on all CPUs
+register write fioff 0x89abcdef
+register write fooff 0x76543210
+
+register write st0 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}"
+register write st1 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}"
+register write st2 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+register write st3 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}"
+register write st4 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}"
+register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}"
+register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}"
+register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+
+process continue
+
+# CHECK: process continue
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# CHECK-DAG: ftag = 0xa961
+# CHECK-DAG: fop = 0x0033
+# CHECK-DAG: fip = 0x89abcdef
+# CHECK-DAG: fdp = 0x76543210
+
+# CHECK-DAG: st0 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40 }
+# CHECK-DAG: st1 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00 }
+# CHECK-DAG: st2 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+# CHECK-DAG: st3 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 }
+# CHECK-DAG: st4 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f }
+# CHECK-DAG: st5 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff }
+# CHECK-DAG: st6 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff }
+# CHECK-DAG: st7 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 }
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -0,0 +1,36 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/x86-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{.*}} stopped
+
+register read --all
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8884
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted; it should be 0xa963 once fixed
+# CHECK-DAG: ftag = 0x00fe
+# CHECK-DAG: fop = 0x0033
+
+# CHECK-DAG: st0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
+# CHECK-DAG: st1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}
+# CHECK-DAG: st2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: st3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}
+# CHECK-DAG: st4 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}
+# CHECK-DAG: st5 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}
+# CHECK-DAG: st6 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}
+# CHECK-DAG: st7 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+# fdiv (%rbx) gets encoded into 2 bytes, int3 into 1 byte
+print (void*)($pc-3)
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+# TODO: we probably should not split it like this
+print (void*)($fiseg*0x1 + $fioff)
+# CHECK: (void *) $1 = [[FDIV]]
+print &zero
+# CHECK: (uint32_t *) $2 = [[ZERO:0x[0-9a-f]*]]
+print (uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t *) $3 = [[ZERO]]
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -0,0 +1,48 @@
+# REQUIRES: native && target-x86_64
+# RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register write fctrl 0x037b
+register write fstat 0x8884
+# note: this needs to enable all registers for writes to be effective
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register writ

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I have two high-level remarks/questions about this:

- I am surprised that it was not necessary to create a special process plugin 
for this purpose. I have a feeling one will be necessary sooner or later 
because of the need to customize the step/continue/etc. flows. Currently, this 
will probably produce very bizarre if one tries to execute those commands. The 
reason I'm bringing this up is because of the `Target::GetTrace` method being 
added in the next patch. If there is a trace-specific process class, then it 
might make sense for this class to hold the trace object instead of adding the 
GetTrace method on every Target object out there even though most targets will 
have that null. I don't know if that will really be the case, but I think it's 
something worth keeping in mind as you work on the subsequent patches.
- I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The two 
classes are very tightly coupled, so it's not clear to me what is the advantage 
of separating it out this way (one could just move all the relevant methods 
directly into the Trace class. What's the reason for this design?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

[Lldb-commits] [PATCH] D88632: A handful of fixes to lldb's "scan the local filesystem for kexts & kernels" feature

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I don't know much about this code so I've left some inline nits.




Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:505
+  bool is_kernel_filename =
+  file_spec.GetFilename().GetStringRef().startswith("kernel") ||
+  file_spec.GetFilename().GetStringRef().startswith("mach");

Nit: You could assign the StringRef to a temporary so you don't have to repeat 
`file_spec.GetFilename().GetStringRef()` thrice. 



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:680
+  std::string possible_path = kernel_dsym.GetPath();
+  if (!kernel_dsym.GetFilename().GetStringRef().endswith(".dSYM"))
+return false;

`FileSpec` has a `GetFileNameExtension` but this works too. 



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:684-685
+  FileSpec binary_filespec = kernel_dsym;
+  std::string filename = binary_filespec.GetFilename().GetCString();
+  filename.erase(filename.size() - 5, filename.size()); // chop off '.dSYM'
+  binary_filespec.GetFilename() = ConstString(filename);

`FileSpec::GetFileNameStrippingExtension` 



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:716
+  // get rid of '.dSYM'
+  filename.erase(filename.size() - 5, filename.size());
+

`FileSpec::GetFileNameStrippingExtension` 



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:865
+} else {
+  error = ModuleList::GetSharedModule(kern_spec, module_sp, NULL, NULL,
+  NULL);

Huh, how did we end up with `NULL` again, someone ran a clang tidy check over 
lldb that replaces it with `nullptr`. Do we only build this on Darwin maybe? If 
the rest of the file uses `NULL` we can fix that inconsistency in a separate 
commit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88632

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

[Lldb-commits] [PATCH] D88681: [lldb] [Process/NetBSD] Fix reading FIP/FDP registers

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

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

Fix reading FIP/FDP registers to correctly return segment and offset
parts.  On amd64, this roughly matches the Linux behavior of splitting
the 64-bit FIP/FDP into two halves, and putting the higher 32 bits
into f*seg and lower into f*off.  Well, actually we use only 16 bits
of higher half but the CPUs do not seem to handle more than that anyway.


https://reviews.llvm.org/D88681

Files:
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -657,13 +657,13 @@
 reg_value = (uint64_t)m_fpr.fxstate.fx_opcode;
 break;
   case lldb_fiseg_x86_64:
-reg_value = (uint64_t)m_fpr.fxstate.fx_ip.fa_64;
+reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_seg;
 break;
   case lldb_fioff_x86_64:
 reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_off;
 break;
   case lldb_foseg_x86_64:
-reg_value = (uint64_t)m_fpr.fxstate.fx_dp.fa_64;
+reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_seg;
 break;
   case lldb_fooff_x86_64:
 reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_off;
@@ -945,13 +945,13 @@
 m_fpr.fxstate.fx_opcode = reg_value.GetAsUInt16();
 break;
   case lldb_fiseg_x86_64:
-m_fpr.fxstate.fx_ip.fa_64 = reg_value.GetAsUInt64();
+m_fpr.fxstate.fx_ip.fa_32.fa_seg = reg_value.GetAsUInt32();
 break;
   case lldb_fioff_x86_64:
 m_fpr.fxstate.fx_ip.fa_32.fa_off = reg_value.GetAsUInt32();
 break;
   case lldb_foseg_x86_64:
-m_fpr.fxstate.fx_dp.fa_64 = reg_value.GetAsUInt64();
+m_fpr.fxstate.fx_dp.fa_32.fa_seg = reg_value.GetAsUInt64();
 break;
   case lldb_fooff_x86_64:
 m_fpr.fxstate.fx_dp.fa_32.fa_off = reg_value.GetAsUInt32();


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -657,13 +657,13 @@
 reg_value = (uint64_t)m_fpr.fxstate.fx_opcode;
 break;
   case lldb_fiseg_x86_64:
-reg_value = (uint64_t)m_fpr.fxstate.fx_ip.fa_64;
+reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_seg;
 break;
   case lldb_fioff_x86_64:
 reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_off;
 break;
   case lldb_foseg_x86_64:
-reg_value = (uint64_t)m_fpr.fxstate.fx_dp.fa_64;
+reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_seg;
 break;
   case lldb_fooff_x86_64:
 reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_off;
@@ -945,13 +945,13 @@
 m_fpr.fxstate.fx_opcode = reg_value.GetAsUInt16();
 break;
   case lldb_fiseg_x86_64:
-m_fpr.fxstate.fx_ip.fa_64 = reg_value.GetAsUInt64();
+m_fpr.fxstate.fx_ip.fa_32.fa_seg = reg_value.GetAsUInt32();
 break;
   case lldb_fioff_x86_64:
 m_fpr.fxstate.fx_ip.fa_32.fa_off = reg_value.GetAsUInt32();
 break;
   case lldb_foseg_x86_64:
-m_fpr.fxstate.fx_dp.fa_64 = reg_value.GetAsUInt64();
+m_fpr.fxstate.fx_dp.fa_32.fa_seg = reg_value.GetAsUInt64();
 break;
   case lldb_fooff_x86_64:
 m_fpr.fxstate.fx_dp.fa_32.fa_off = reg_value.GetAsUInt32();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88682: [lldb] [Process/NetBSD] Fix crash on unsupported i386 regs

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

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

Fix crash due to overeager assert on translating 32-bit to 64-bit
register constants.  Instead, just return an invalid register index
and let it be ignored just like native requests for unsupported
registers.

While at it, fill the missing translations.


https://reviews.llvm.org/D88682

Files:
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -375,6 +375,15 @@
   case lldb_ymm6_i386:
   case lldb_ymm7_i386:
 return lldb_ymm0_x86_64 + regnum - lldb_ymm0_i386;
+  case lldb_bnd0_i386:
+  case lldb_bnd1_i386:
+  case lldb_bnd2_i386:
+  case lldb_bnd3_i386:
+return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
+  case lldb_bndcfgu_i386:
+return lldb_bndcfgu_x86_64;
+  case lldb_bndstatus_i386:
+return lldb_bndstatus_x86_64;
   case lldb_dr0_i386:
   case lldb_dr1_i386:
   case lldb_dr2_i386:
@@ -385,8 +394,7 @@
   case lldb_dr7_i386:
 return lldb_dr0_x86_64 + regnum - lldb_dr0_i386;
   default:
-assert(false && "Unhandled i386 register.");
-return 0;
+return -1;
   }
 }
 


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -375,6 +375,15 @@
   case lldb_ymm6_i386:
   case lldb_ymm7_i386:
 return lldb_ymm0_x86_64 + regnum - lldb_ymm0_i386;
+  case lldb_bnd0_i386:
+  case lldb_bnd1_i386:
+  case lldb_bnd2_i386:
+  case lldb_bnd3_i386:
+return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
+  case lldb_bndcfgu_i386:
+return lldb_bndcfgu_x86_64;
+  case lldb_bndstatus_i386:
+return lldb_bndstatus_x86_64;
   case lldb_dr0_i386:
   case lldb_dr1_i386:
   case lldb_dr2_i386:
@@ -385,8 +394,7 @@
   case lldb_dr7_i386:
 return lldb_dr0_x86_64 + regnum - lldb_dr0_i386;
   default:
-assert(false && "Unhandled i386 register.");
-return 0;
+return -1;
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88583: [lldb] [test/Register] Add read/write tests for x87 regs

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

mgorny added a reviewer: teemperor.
mgorny added a subscriber: teemperor.
mgorny added a comment.

@teemperor would you perhaps be interested in testing this on macos before I 
commit it?


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

https://reviews.llvm.org/D88583

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

Re: [Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Walter via lldb-commits]

> - I am surprised that it was not necessary to create a special process
plugin for this purpose. I have a feeling one will be necessary sooner or
later because of the need to customize the step/continue/etc. flows.
Currently, this will probably produce very bizarre if one tries to execute
those commands. The reason I'm bringing this up is because of the
`Target::GetTrace` method being added in the next patch. If there is a
trace-specific process class, then it might make sense for this class to
hold the trace object instead of adding the GetTrace method on every Target
object out there even though most targets will have that null. I don't know
if that will really be the case, but I think it's something worth keeping
in mind as you work on the subsequent patches.

Very good remark. Probably we'll end up doing that when we start
implementing reverse debugging. The tricky thing we'll need to solve in the
future is seamlessly transition between a trace and a live process. For
example, when reverse debugging a live process, you might be stopped at a
breakpoint in the trace, then you do "continue", it reaches the end of the
trace, and then it resumes the live process. I still haven't decided what
we'll propose for this, but probably we'll have to change a lot of the
current code to make that happen nicely.

> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The
two classes are very tightly coupled, so it's not clear to me what is the
advantage of separating it out this way (one could just move all the
relevant methods directly into the Trace class. What's the reason for this
design?

Well, there's definitely coupling, but there are two reasons. One is that
the Trace of a live process won't need any parsing. Parsing is only used
when loading a trace from a json file. That makes parsing one of the two
main ways to create a Trace. And besides, I think that the single
responsibility principle is good to follow. The Parser class does the
parsing, and the Trace class holds an actual trace.

Il giorno gio 1 ott 2020 alle ore 07:12 Pavel Labath via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> labath added a comment.
>
> I have two high-level remarks/questions about this:
>
> - I am surprised that it was not necessary to create a special process
> plugin for this purpose. I have a feeling one will be necessary sooner or
> later because of the need to customize the step/continue/etc. flows.
> Currently, this will probably produce very bizarre if one tries to execute
> those commands. The reason I'm bringing this up is because of the
> `Target::GetTrace` method being added in the next patch. If there is a
> trace-specific process class, then it might make sense for this class to
> hold the trace object instead of adding the GetTrace method on every Target
> object out there even though most targets will have that null. I don't know
> if that will really be the case, but I think it's something worth keeping
> in mind as you work on the subsequent patches.
> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The two
> classes are very tightly coupled, so it's not clear to me what is the
> advantage of separating it out this way (one could just move all the
> relevant methods directly into the Trace class. What's the reason for this
> design?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D85705/new/
>
> https://reviews.llvm.org/D85705
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D85705#2305994 , @labath wrote:

> I have two high-level remarks/questions about this:
>
> - I am surprised that it was not necessary to create a special process plugin 
> for this purpose. I have a feeling one will be necessary sooner or later 
> because of the need to customize the step/continue/etc. flows. Currently, 
> this will probably produce very bizarre if one tries to execute those 
> commands. The reason I'm bringing this up is because of the 
> `Target::GetTrace` method being added in the next patch. If there is a 
> trace-specific process class, then it might make sense for this class to hold 
> the trace object instead of adding the GetTrace method on every Target object 
> out there even though most targets will have that null. I don't know if that 
> will really be the case, but I think it's something worth keeping in mind as 
> you work on the subsequent patches.

Eventually we should probably have a HistoryProcess class, much like the 
HistoryThread. This new class could then be used as a base class for the 
ProcessMachCore and ProcessELFCore and ProcessMinidump. This base class can 
then stub out the flow control (continue, step, stop etc) and all other virtual 
methods that are stubbed out in these classes. It can also add accessors for 
adding HistoryThread objects if needed. So we should probable move this way.

When we have a live process, we will use the current process when we can, but 
the process and target will need to coordinate to be able to know if we have 
reversed stepped into the past so that the flow control (forward/reverse 
continue, forward/reverse step) can do the right things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D87868#2305345 , @labath wrote:

> In D87868#2304680 , @clayborg wrote:
>
>> That is possible, though how do we figure out the syscall enumeration for 
>> the mmap syscall reliably for a given system? And if we are remote 
>> debugging? Seems like we are signing up for architecture specific syscalls 
>> on every different architecture. Not sure I would go that route, but I am 
>> open to seeing a solution before passing judgement.
>
> Well.. we already need to "know" the right target-specific values for the 
> various PROT_ and MAP_ flags, so I don't think including the syscall number 
> (and the syscall opcode) would not be too much of a stretch.
>
>>> Note that this would not need to be implemented in the lldb client. This 
>>> sort of thing would be natural to implement in lldb server in response to 
>>> the `_M` packet. There it would be easy to encode the abi details needed to 
>>> issue a syscall. The client already prefers this packet, and the existing 
>>> code could remain as a fallback for platforms not implementing it.
>>
>> There is no code currently that does any run control down inside of 
>> lldb-server and I would like to keep it that way if possible. debugserver 
>> can do it only because we have the task port to the program we are debugging 
>> and we can call a function in the debugserver process that does the memory 
>> allocation in the debug process. Having this done in lldb-server would 
>> require lldb-server to perform run control by changing register values, 
>> running the syscall, stopping at a breakpoint to after the syscall has run, 
>> removing that breakpoint only if it didn't exist already. Seems like a lot 
>> of dangerous flow control that we won't be able to see if anything goes 
>> wrong. Right now if we are doing it all through the GDB remote protocol, we 
>> can see exactly how things go wrong in the packet log, but now it would be a 
>> mystery if things go wrong.
>
> I do share that sentiment, and if the setup needed for this would be anything 
> like the mmap dance, I wouldn't even try it. The only reason I brought this 
> up is because I expect the syscall routine to be much simpler (like, maybe 
> simpler than the instruction emulation routine that we do on arm). There's no 
> need to set breakpoints, as we're just PTRACE_SINGLESTEPing over a single 
> instruction (unless we're on arm of course). So the implementation would be 
> something like:
>
> - save all registers
> - find first executable page
> - write "int 0x80" to the first two bytes
> - setup registers for a syscall (including pointing pc to the int 0x80 
> instruction)
> - PTRACE_SINGLESTEP+waitpid
> - fetch result from %rax
> - restore bytes overwritten by the int 0x80
> - restore all registers
>
> I don't think this would be too complicated, though it would obviously have 
> to be done with a steady hand. In fact, one of the original use cases for the 
> linux ptrace syscall was the ability for the tracer to track/replace/emulate 
> syscalls done by the traced process. Unfortunately, the intended use was a 
> bit different (things like qemu and user-mode linux) and it assumes the 
> tracer wants to modify a syscall that the tracee already wanted to make (and 
> not inject a new one "out of the blue"). This means we cannot use the 
> existing support for that. That said, I think I've managed to convince the 
> NetBSD folks to include out-of-blue syscall support in their ptrace 
> implementation. Maybe I should try the same for linux... :P

If we can truly get away with the single stepping over a single instructions, 
then I could see this working. Many ARM chips don't support hardware single 
stepping, so software single stepping would need to be used and that would 
complicate things. This is arch specific, so each arch would need to know how 
to do this. Happy to review a patch and see if we can make it work. Might be 
nice to have a setting for this in case it causes problems to stop it from 
being attempted:

  (lldb) setting set plugin.process.gdb-remote.use-allocate-memory-packets false




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87868

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

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 21 inline comments as done.
wallace added inline comments.



Comment at: lldb/include/lldb/Target/Target.h:32
 #include "lldb/Target/ThreadSpec.h"
+#include "lldb/Target/Trace.h"
 #include "lldb/Utility/ArchSpec.h"

clayborg wrote:
> You don't need this as the only "Trace" in this file is the "TraceSP" which 
> is a forward declaration. You will need to add it to the Target.cpp file 
> though.
It's needed for accessing TraceDumpOptions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 295655.
wallace added a comment.

comments addressed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.cpp
  lldb/test/API/commands/trace/TestTraceDump.py
  lldb/test/API/commands/trace/intelpt-trace/trace2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace2.json
@@ -0,0 +1,53 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 11,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+},
+{
+  "pid": 2,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 21,
+  "traceFile": "3842849.trace"
+},
+{
+  "tid": 22,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceDump.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDump.py
@@ -0,0 +1,49 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDump(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+self.src_dir = self.getSourceDir()
+
+def testDumpMissingTrace(self):
+self.expect("target create " + os.path.join(self.src_dir, 'intelpt-trace', 'a.out'))
+self.expect("trace dump", substrs=["error: no trace data in this target"])
+
+def testDumpTrace(self):
+self.expect("trace load " + os.path.join(self.src_dir, "intelpt-trace", "trace2.json"))
+
+# An invalid thread id should show an error message
+self.expect("trace dump -t 5678", substrs=['error: the thread id "5678" does not exist in this target'])
+
+# Only the specified thread should be printed
+self.expect("trace dump -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"Modules:\n  uuid: 6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A, load address: 0x0040, file: .*intelpt-trace/a.out"])
+
+# Verbose should output the entire JSON settings besides the thread specific information
+self.expect("trace dump -t 22 -v",
+  substrs=["Settings", "3842849.trace", "intel-pt", "Settings directory"],
+  patterns=["pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# In this case all threads should be printed
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+self.expect("trace dump -t 21 -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# We switch targets and check the threads of this target
+self.dbg.SetSelectedTarget(self.dbg.FindTargetWithProcessID(1))
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '1', tid: '11', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
Index: lldb/source/Target/TraceSettingsParser.cpp
==

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 295656.
wallace added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.cpp
  lldb/test/API/commands/trace/TestTraceDump.py
  lldb/test/API/commands/trace/intelpt-trace/trace2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace2.json
@@ -0,0 +1,53 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 11,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+},
+{
+  "pid": 2,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 21,
+  "traceFile": "3842849.trace"
+},
+{
+  "tid": 22,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceDump.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDump.py
@@ -0,0 +1,49 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDump(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+self.src_dir = self.getSourceDir()
+
+def testDumpMissingTrace(self):
+self.expect("target create " + os.path.join(self.src_dir, 'intelpt-trace', 'a.out'))
+self.expect("trace dump", substrs=["error: no trace data in this target"])
+
+def testDumpTrace(self):
+self.expect("trace load " + os.path.join(self.src_dir, "intelpt-trace", "trace2.json"))
+
+# An invalid thread id should show an error message
+self.expect("trace dump -t 5678", substrs=['error: the thread id "5678" does not exist in this target'])
+
+# Only the specified thread should be printed
+self.expect("trace dump -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"Modules:\n  uuid: 6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A, load address: 0x0040, file: .*intelpt-trace/a.out"])
+
+# Verbose should output the entire JSON settings besides the thread specific information
+self.expect("trace dump -t 22 -v",
+  substrs=["Settings", "3842849.trace", "intel-pt", "Settings directory"],
+  patterns=["pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# In this case all threads should be printed
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+self.expect("trace dump -t 21 -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# We switch targets and check the threads of this target
+self.dbg.SetSelectedTarget(self.dbg.FindTargetWithProcessID(1))
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '1', tid: '11', trace file: .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
Index: lldb/source/Target/TraceSettingsParser.cpp
===
--- lldb/s

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[Greg Clayton via Phabricator via lldb-commits]

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

Just one nit about forward declaring the TraceDumpOptions.




Comment at: lldb/include/lldb/Target/Target.h:32
 #include "lldb/Target/ThreadSpec.h"
+#include "lldb/Target/Trace.h"
 #include "lldb/Utility/ArchSpec.h"

Add TraceDumpOptions to lldb-forward.h and then this won't be needed here. 
Anytime you only have pointers or references to objects in headers, we should 
use forward declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

Re: [Lldb-commits] [lldb] 1b1d981 - Revert "Revert "Add the ability to write target stop-hooks using the ScriptInterpreter.""

[Jim Ingham via lldb-commits]

Thanks for the info…

We’re running in sync mode, so for Continue to return before the process is all 
the way stopped, Process::ResumeSynchronous() must be bobbling the case where a 
stop event that resumes the process comes in.  The only way I can see that 
happening is if Process::PrivateResume can return before setting the private 
state to eStateRunning.  But only when something gets a chance to time out 
somewhere along the line.

I’ve been looking but I haven’t found anything relevant along this code path.  
There is one hard-coded timeout along this path, the 5 second wait between 
sending the eBroadcastBitAsyncContinue with the continue packet to the 
gdb-remote async thread and receiving the ack back (in 
ProcessGDBRemote::DoResume).  But if that timed out it would write a 
“gdb-remote process” log message: "Resume timed out”.  The process log is on, I 
see other output from it, but I don’t see that message in the failure 
transcript.

Weird.

Jim

> On Oct 1, 2020, at 5:26 AM, Raphael “Teemperor” Isemann  
> wrote:
> 
> +1, I have two machines with very similar setup where only the one that is 
> under heavy load sees the test failures.
> 
> - Raphael
> 
>> On 1 Oct 2020, at 14:24, Pavel Labath  wrote:
>> 
>> On 30/09/2020 23:21, Jim Ingham wrote:
>>> The test doesn’t seem to be flakey in the “run it a bunch of times and
>>> it will eventually fail” type flakey.  I ran the test 200 times on my
>>> machine and didn’t get a failure.
>> 
>> Actually, it seems like exactly the typical kind of flaky test to me --
>> it mostly works when run on its own, but starts failing as soon as the
>> system comes under load.
>> 
>> It didn't fail for me either for over 100 iterations. However, as soon
>> as I cranked up the cpu load (compiling llvm is good at that), it failed
>> almost immediately.
>> 
>> It also doesn't seem to be related to the way the stop hook resumes the
>> process.
>> 
>> is one example where the auto_continue version of the test fails, and I
>> have seen both tests fail on my machine.
>> 
>> I have some traces of failing and successful runs of the test (will send
>> them to you in a private email). I didn't dive too deeply, but the
>> problem does not seem to be related to python stop hooks. It looks more
>> like a general stop hook bug, which we've had problems with in the past.
>> 
>> The problems seems to be that the process.Continue() on the main thread
>> returns early, and so the subsequent checks (for the topmost frame etc.)
>> execute concurrently with the "step out" action. In the "Failure" file
>> I'll send you you can see that (line 9222) SBFrame::GetFunctionName is
>> called before the inferior process stops in the main function (the
>> processing of that happens immediately after that line, on the
>> "intern-state" thread).
>> 
>> pl
> 

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

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[Greg Clayton via Phabricator via lldb-commits]

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

Some active discussion on the Trace RFC right now that might change the outcome 
of this patch. Holding off accepting until we resolve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

[Lldb-commits] [lldb] 15ea45f - [lldb] Skip unique_ptr import-std-module tests on Linux

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-10-01T23:04:36+02:00
New Revision: 15ea45f16b261521e3251b4ff0bceaadf31a4515

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

LOG: [lldb] Skip unique_ptr import-std-module tests on Linux

This seems to fail on ubuntu 18.04.5 with Clang 9 due to:

Error output:
error: Couldn't lookup symbols:
  std::__1::default_delete::operator()(int) const

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py

lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
 
b/lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
index fafb29333924..9f698af7e1b4 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
@@ -13,6 +13,7 @@ class TestUniquePtrDbgInfoContent(TestBase):
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIfLinux # s.reset() causes link errors on ubuntu 18.04/Clang 9
 def test(self):
 self.build()
 

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
index 7bd391243734..a3761ade04e3 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
@@ -13,6 +13,7 @@ class TestUniquePtr(TestBase):
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIfLinux # s.reset() causes link errors on ubuntu 18.04/Clang 9
 def test(self):
 self.build()
 



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

[Lldb-commits] [PATCH] D88704: [lldb] Fix bug in fallback logic for finding the resource directory.

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Both of the if-clauses modify the `raw_path` variable and the 
`SharedFrameworks` variant wasn't properly resetting the variable. Avoid future 
bugs like that by always resetting the variable.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D88704

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -137,14 +137,12 @@
 FileSystem::Instance().Resolve(file_spec);
 return true;
   }
-  raw_path = lldb_shlib_spec.GetPath();
 }
-raw_path.resize(rev_it - r_end);
-  } else {
-raw_path.resize(rev_it - r_end);
   }
 
   // Fall back to the Clang resource directory inside the framework.
+  raw_path = lldb_shlib_spec.GetPath();
+  raw_path.resize(rev_it - r_end);
   raw_path.append("LLDB.framework/Resources/Clang");
   file_spec.GetDirectory().SetString(raw_path.c_str());
   FileSystem::Instance().Resolve(file_spec);


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -137,14 +137,12 @@
 FileSystem::Instance().Resolve(file_spec);
 return true;
   }
-  raw_path = lldb_shlib_spec.GetPath();
 }
-raw_path.resize(rev_it - r_end);
-  } else {
-raw_path.resize(rev_it - r_end);
   }
 
   // Fall back to the Clang resource directory inside the framework.
+  raw_path = lldb_shlib_spec.GetPath();
+  raw_path.resize(rev_it - r_end);
   raw_path.append("LLDB.framework/Resources/Clang");
   file_spec.GetDirectory().SetString(raw_path.c_str());
   FileSystem::Instance().Resolve(file_spec);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

What's the difference between design/ and architecture/? These sounds like 
synonyms to me.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D88516

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

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D88516#2307486 , @aprantl wrote:

> What's the difference between design/ and architecture/? These sounds like 
> synonyms to me.

Good point, that page should be moved under the new section as well. Maybe we 
should call the section "Architecture" and rename that page to "Overview" when 
nested underneath?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D88516

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

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D88516

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

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Ah, Adrian is right, you should probably move Architecture to Design.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D88516

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

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I like the name Design better than Architecture for the top level.  For 
instance the structureddataplugins is not really Architecture, it's the design 
of a fairly small component of lldb


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D88516

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

Re: [Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

[Jim Ingham via lldb-commits]

Architecture is more specific to the structural aspect of a thing, whereas 
Design is more general.  For instance, the rules for the SB API don’t really 
seem like a description of an Architecture, but more some general principles 
for doing the thing.

Jim

> On Oct 1, 2020, at 4:25 PM, Adrian Prantl via Phabricator 
>  wrote:
> 
> aprantl added a comment.
> 
> What's the difference between design/ and architecture/? These sounds like 
> synonyms to me.
> 
> 
> Repository:
>  rLLDB LLDB
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D88516/new/
> 
> https://reviews.llvm.org/D88516
> 

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

[Lldb-commits] [PATCH] D88632: A handful of fixes to lldb's "scan the local filesystem for kexts & kernels" feature

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 295713.
jasonmolenda added a comment.

Incorporate Jonas' suggestions, including changing the NULLs to nullptr's in 
the section of code I was changing right now.  Also found an unintentional 
shadowing of module_sp in GetSharedModuleKernel (that was present before my 
patchset) and fixed that; it works either way, but it's not correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88632

Files:
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td

Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
@@ -1,10 +1,6 @@
 include "../../../../include/lldb/Core/PropertiesBase.td"
 
 let Definition = "platformdarwinkernel" in {
-  def SearchForKexts: Property<"search-locally-for-kexts", "Boolean">,
-Global,
-DefaultTrue,
-Desc<"Automatically search for kexts on the local system when doing kernel debugging.">;
   def KextDirectories: Property<"kext-directories", "FileSpecList">,
 DefaultStringValue<"">,
 Desc<"Directories/KDKs to search for kexts in when starting a kernel debug session.">;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
@@ -126,7 +126,30 @@
 
   // Returns true if there is a .dSYM bundle next to the kernel
   static bool
-  KernelHasdSYMSibling(const lldb_private::FileSpec &kext_bundle_filepath);
+  KernelHasdSYMSibling(const lldb_private::FileSpec &kernel_filepath);
+
+  // Returns true if there is a .dSYM bundle with NO kernel binary next to it
+  static bool KerneldSYMHasNoSiblingBinary(
+  const lldb_private::FileSpec &kernel_dsym_filepath);
+
+  // Given a dsym_bundle argument ('.../foo.dSYM'), return a FileSpec
+  // with the binary inside it ('.../foo.dSYM/Contents/Resources/DWARF/foo').
+  // A dSYM bundle may have multiple DWARF binaries in them, so a vector
+  // of matches is returned.
+  static std::vector
+  GetDWARFBinaryInDSYMBundle(lldb_private::FileSpec dsym_bundle);
+
+  lldb_private::Status
+  GetSharedModuleKext(const lldb_private::ModuleSpec &module_spec,
+  lldb_private::Process *process, lldb::ModuleSP &module_sp,
+  const lldb_private::FileSpecList *module_search_paths_ptr,
+  lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
+
+  lldb_private::Status GetSharedModuleKernel(
+  const lldb_private::ModuleSpec &module_spec,
+  lldb_private::Process *process, lldb::ModuleSP &module_sp,
+  const lldb_private::FileSpecList *module_search_paths_ptr,
+  lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
 
   lldb_private::Status
   ExamineKextForMatchingUUID(const lldb_private::FileSpec &kext_bundle_path,
@@ -170,6 +193,13 @@
   // on local
   // filesystem, with
   // dSYMs next to them
+  KernelBinaryCollection m_kernel_dsyms_no_binaries;  // list of kernel
+  // dsyms with no
+  // binaries next to
+  // them
+  KernelBinaryCollection m_kernel_dsyms_yaas; // list of kernel
+  // .dSYM.yaa files
+
   lldb_private::LazyBool m_ios_debug_session;
 
   PlatformDarwinKernel(const PlatformDarwinKernel &) = delete;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -199,13 +199,6 @@
 
   virtual ~PlatformDarwinKernelProperties() {}
 
-  bool GetSearchForKexts() const {
-const uint32_t idx = ePropertySearchForKexts;
-return m_collection_sp->GetPropertyAtIndexAsBoolean(
-NULL, idx,
-g_platformdarwinkernel_properties[idx].default_uint_value != 0);
-  }
-
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
 const OptionValueFileSpecList *option_value =
@@ -245,14 +238,12 @@
   m_name_to_kext_path_map_with_dsyms(),
   

[Lldb-commits] [lldb] a1e9792 - Have kernel binary scanner load dSYMs as binary+dSYM if best thing found

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2020-10-01T18:55:37-07:00
New Revision: a1e97923a025d09934b557ca4343d8e4b5a9973d

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

LOG: Have kernel binary scanner load dSYMs as binary+dSYM if best thing found

lldb's PlatforDarwinKernel scans the local filesystem (well known
locations, plus user-specified directories) for kernels and kexts
when doing kernel debugging, and loads them automatically.  Sometimes
kernel developers want to debug with *only* a dSYM, in which case they
give lldb the DWARF binary + the dSYM as a binary and symbol file.
This patch adds code to lldb to do this automatically if that's the
best thing lldb can find.

A few other bits of cleanup in PlatformDarwinKernel that I undertook
at the same time:

1. Remove the 'platform.plugin.darwin-kernel.search-locally-for-kexts'
setting.  When I added the local filesystem index at start of kernel
debugging, I thought people might object to the cost of the search
and want a way to disable it.  No one has.

2. Change the behavior of
'plugin.dynamic-loader.darwin-kernel.load-kexts' setting so it does
not disable the local filesystem scan, or use of the local filesystem
binaries.

3. PlatformDarwinKernel::GetSharedModule into GetSharedModuleKext and
GetSharedModuleKernel for easier readability & maintenance.

4. Added accounting of .dSYM.yaa files (an archive format akin to tar)
that I come across during the scan.  I'm not using these for now; it
would be very expensive to expand the archives & see if the UUID matches
what I'm searching for.


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

Added: 


Modified: 

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
index 68a0335682d3..d0d5a99b28ed 100644
--- 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -517,12 +517,8 @@ 
DynamicLoaderDarwinKernel::DynamicLoaderDarwinKernel(Process *process,
   Status error;
   PlatformSP platform_sp(
   Platform::Create(PlatformDarwinKernel::GetPluginNameStatic(), error));
-  // Only select the darwin-kernel Platform if we've been asked to load kexts.
-  // It can take some time to scan over all of the kext info.plists and that
-  // shouldn't be done if kext loading is explicitly disabled.
-  if (platform_sp.get() && GetGlobalProperties()->GetLoadKexts()) {
+  if (platform_sp.get())
 process->GetTarget().SetPlatform(platform_sp);
-  }
 }
 
 // Destructor

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
index f6c0f262a379..54f49601e811 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -199,13 +199,6 @@ class PlatformDarwinKernelProperties : public Properties {
 
   virtual ~PlatformDarwinKernelProperties() {}
 
-  bool GetSearchForKexts() const {
-const uint32_t idx = ePropertySearchForKexts;
-return m_collection_sp->GetPropertyAtIndexAsBoolean(
-NULL, idx,
-g_platformdarwinkernel_properties[idx].default_uint_value != 0);
-  }
-
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
 const OptionValueFileSpecList *option_value =
@@ -245,14 +238,12 @@ PlatformDarwinKernel::PlatformDarwinKernel(
   m_name_to_kext_path_map_with_dsyms(),
   m_name_to_kext_path_map_without_dsyms(), m_search_directories(),
   m_search_directories_no_recursing(), m_kernel_binaries_with_dsyms(),
-  m_kernel_binaries_without_dsyms(),
-  m_ios_debug_session(is_ios_debug_session)
+  m_kernel_binaries_without_dsyms(), m_kernel_dsyms_no_binaries(),
+  m_kernel_dsyms_yaas(), m_ios_debug_session(is_ios_debug_session)
 
 {
-  if (GetGlobalProperties()->GetSearchForKexts()) {
-CollectKextAndKernelDirectories();
-SearchForKextsAndKernelsRecursively();
-  }
+  CollectKextAndKernelDirectories();
+  SearchForKextsAndKernelsRecursively();
 }
 
 /// Destructor.
@@ -293,6 +284,10 @@ void PlatformDarwinKernel::GetStatus(Stream &strm) {
   (int)m_kernel_binaries_with_dsyms.size());
   strm.Printf(" Number of Kernel binaries without dSYMs indexed: %d\n",
   (int)m_kernel_binaries_without_dsyms.size());

[Lldb-commits] [PATCH] D88632: A handful of fixes to lldb's "scan the local filesystem for kexts & kernels" feature

[Jason Molenda via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1e97923a025: Have kernel binary scanner load dSYMs as 
binary+dSYM if best thing found (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88632

Files:
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td

Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
@@ -1,10 +1,6 @@
 include "../../../../include/lldb/Core/PropertiesBase.td"
 
 let Definition = "platformdarwinkernel" in {
-  def SearchForKexts: Property<"search-locally-for-kexts", "Boolean">,
-Global,
-DefaultTrue,
-Desc<"Automatically search for kexts on the local system when doing kernel debugging.">;
   def KextDirectories: Property<"kext-directories", "FileSpecList">,
 DefaultStringValue<"">,
 Desc<"Directories/KDKs to search for kexts in when starting a kernel debug session.">;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
@@ -126,7 +126,30 @@
 
   // Returns true if there is a .dSYM bundle next to the kernel
   static bool
-  KernelHasdSYMSibling(const lldb_private::FileSpec &kext_bundle_filepath);
+  KernelHasdSYMSibling(const lldb_private::FileSpec &kernel_filepath);
+
+  // Returns true if there is a .dSYM bundle with NO kernel binary next to it
+  static bool KerneldSYMHasNoSiblingBinary(
+  const lldb_private::FileSpec &kernel_dsym_filepath);
+
+  // Given a dsym_bundle argument ('.../foo.dSYM'), return a FileSpec
+  // with the binary inside it ('.../foo.dSYM/Contents/Resources/DWARF/foo').
+  // A dSYM bundle may have multiple DWARF binaries in them, so a vector
+  // of matches is returned.
+  static std::vector
+  GetDWARFBinaryInDSYMBundle(lldb_private::FileSpec dsym_bundle);
+
+  lldb_private::Status
+  GetSharedModuleKext(const lldb_private::ModuleSpec &module_spec,
+  lldb_private::Process *process, lldb::ModuleSP &module_sp,
+  const lldb_private::FileSpecList *module_search_paths_ptr,
+  lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
+
+  lldb_private::Status GetSharedModuleKernel(
+  const lldb_private::ModuleSpec &module_spec,
+  lldb_private::Process *process, lldb::ModuleSP &module_sp,
+  const lldb_private::FileSpecList *module_search_paths_ptr,
+  lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
 
   lldb_private::Status
   ExamineKextForMatchingUUID(const lldb_private::FileSpec &kext_bundle_path,
@@ -170,6 +193,13 @@
   // on local
   // filesystem, with
   // dSYMs next to them
+  KernelBinaryCollection m_kernel_dsyms_no_binaries;  // list of kernel
+  // dsyms with no
+  // binaries next to
+  // them
+  KernelBinaryCollection m_kernel_dsyms_yaas; // list of kernel
+  // .dSYM.yaa files
+
   lldb_private::LazyBool m_ios_debug_session;
 
   PlatformDarwinKernel(const PlatformDarwinKernel &) = delete;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -199,13 +199,6 @@
 
   virtual ~PlatformDarwinKernelProperties() {}
 
-  bool GetSearchForKexts() const {
-const uint32_t idx = ePropertySearchForKexts;
-return m_collection_sp->GetPropertyAtIndexAsBoolean(
-NULL, idx,
-g_platformdarwinkernel_properties[idx].default_uint_value != 0);
-  }
-
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
 const OptionValueFileSpecList *option_value =
@@ -245,14 +238,12 @@
   m_name_to_kext_path_map_with_dsyms(),
   m_name_to_kext_path_map_without_dsy