This test has been flakey on our bot:
https://lab.llvm.org/buildbot/#/builders/18/builds/20410
```
======================================================================
FAIL: test_extra_launch_commands (TestDAP_launch.TestDAP_launch)
Tests the "launchCommands" with extra launching settings
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py", line 482, in test_extra_launch_commands
self.verify_commands("stopCommands", output, stopCommands)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 228, in verify_commands
self.assertTrue(
AssertionError: False is not true : verify 'frame variable' found in console output for 'stopCommands'
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
----------------------------------------------------------------------
```
Likely a timing issue waiting for the command output on a slower
machine.
General tracking issue - https://github.com/llvm/llvm-project/issues/137660
Reapply "[lldb] Update JSONTransport to use MainLoop for reading."
(#152155)
This reverts commit cd40281685f642ad879e33f3fda8d1faa136ebf4.
This also includes some updates to try to address the platforms with
failing tests.
I updated the JSONTransport and tests to use std::function instead of
llvm:unique_function. I think the tests were failing due to the
unique_function not being moved correctly in the loop on some platforms.
This is a major change on how we represent nested name qualifications in
the AST.
* The nested name specifier itself and how it's stored is changed. The
prefixes for types are handled within the type hierarchy, which makes
canonicalization for them super cheap, no memory allocation required.
Also translating a type into nested name specifier form becomes a no-op.
An identifier is stored as a DependentNameType. The nested name
specifier gains a lightweight handle class, to be used instead of
passing around pointers, which is similar to what is implemented for
TemplateName. There is still one free bit available, and this handle can
be used within a PointerUnion and PointerIntPair, which should keep
bit-packing aficionados happy.
* The ElaboratedType node is removed, all type nodes in which it could
previously apply to can now store the elaborated keyword and name
qualifier, tail allocating when present.
* TagTypes can now point to the exact declaration found when producing
these, as opposed to the previous situation of there only existing one
TagType per entity. This increases the amount of type sugar retained,
and can have several applications, for example in tracking module
ownership, and other tools which care about source file origins, such as
IWYU. These TagTypes are lazily allocated, in order to limit the
increase in AST size.
This patch offers a great performance benefit.
It greatly improves compilation time for
[stdexec](https://github.com/NVIDIA/stdexec). For one datapoint, for
`test_on2.cpp` in that project, which is the slowest compiling test,
this patch improves `-c` compilation time by about 7.2%, with the
`-fsyntax-only` improvement being at ~12%.
This has great results on compile-time-tracker as well:
This patch also further enables other optimziations in the future, and
will reduce the performance impact of template specialization resugaring
when that lands.
It has some other miscelaneous drive-by fixes.
About the review: Yes the patch is huge, sorry about that. Part of the
reason is that I started by the nested name specifier part, before the
ElaboratedType part, but that had a huge performance downside, as
ElaboratedType is a big performance hog. I didn't have the steam to go
back and change the patch after the fact.
There is also a lot of internal API changes, and it made sense to remove
ElaboratedType in one go, versus removing it from one type at a time, as
that would present much more churn to the users. Also, the nested name
specifier having a different API avoids missing changes related to how
prefixes work now, which could make existing code compile but not work.
How to review: The important changes are all in
`clang/include/clang/AST` and `clang/lib/AST`, with also important
changes in `clang/lib/Sema/TreeTransform.h`.
The rest and bulk of the changes are mostly consequences of the changes
in API.
PS: TagType::getDecl is renamed to `getOriginalDecl` in this patch, just
for easier to rebasing. I plan to rename it back after this lands.
Fixes#136624
Fixes https://github.com/llvm/llvm-project/issues/43179
Fixes https://github.com/llvm/llvm-project/issues/68670
Fixes https://github.com/llvm/llvm-project/issues/92757
Resolves#141955
- Adds data to breakpoints `Source` object, in order for assembly
breakpoints, which rely on a temporary `sourceReference` value, to be
able to resolve in future sessions like normal path+line breakpoints
- Adds optional `instructions_offset` parameter to `BreakpointResolver`
This has been very flakey lately:
======================================================================
ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory)
Tests the 'writeMemory' request
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py", line 202, in test_writeMemory
mem_response = self.writeMemory(
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 545, in writeMemory
response = self.dap_server.request_writeMemory(
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 850, in request_writeMemory
return self.send_recv(command_dict)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 403, in send_recv
raise ValueError(desc)
ValueError: no response for "writeMemory"
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
======================================================================
ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory)
Tests the 'writeMemory' request
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2067, in tearDown
Base.tearDown(self)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1105, in tearDown
hook() # try the plain call and hope it works
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 486, in cleanup
self.dap_server.request_disconnect(terminateDebuggee=True)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 799, in request_disconnect
return self.send_recv(command_dict)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 397, in send_recv
self.send_packet(command)
File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 349, in send_packet
self.send.flush()
BrokenPipeError: [Errno 32] Broken pipe
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
----------------------------------------------------------------------
General tracking issue - https://github.com/llvm/llvm-project/issues/137660
This updates JSONTransport to use a MainLoop for reading messages.
This also allows us to read in larger chunks than we did previously.
With the event driven reading operations we can read in chunks and store
the contents in an internal buffer. Separately we can parse the buffer
and split the contents up into messages.
Our previous version approach would read a byte at a time, which is less
efficient.
Currently, the "stopped" event returned when a breakpoint is hit will
always return only the ID of first breakpoint returned from
`GetStopReasonDataAtIndex`. This is slightly different from the
behaviour in `lldb`, where multiple breakpoints can exist at a single
instruction address and all are returned as part of the stop reason when
that address is hit.
This patch allows all multiple hit breakpoints to be returned in the
"stopped" event, both in the hitBreakpointIds field and in the
description, using the same formatting as lldb e.g. "breakpoint 1.1
2.1". I'm not aware of any effect this will have on debugger plugins; as
far as I can tell, it makes no difference within the VS Code UI - this
just fixes a minor issue encountered while writing an `lldb-dap` backend
for Dexter.
This updates all the existing memory reference fields to use
`lldb::addr_t` directly.
A few places were using `std::string` and decoding the value in the
request handler and other places had unique ways of parsing addresses.
This unifies all of these references with the `DecodeMemoryReference`
helper in JSONUtils.h.
Additionally, for the types I updated, I tried to simplify the POD types
some and moved default values out of RequestHandlers and into the
protocol POD types.
This adds new protocol types for the 'variables' request.
While implementing this, I removed the '$__lldb_extension' field we
returned on the 'variables' request, since I think all the data can be
retrieved from other DAP requests.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
When there is a function that is inlined at the current program counter.
If you get the current `line_entry` using the program counter's address
it will point to the location of the inline function that may be in
another file. (this is in implicit step-in and should not happen what
step over is called).
Use the current frame to get the `line_entry`
This patch fixes a possible data race between main and event handler
threads. Terminated event can be sent from `Disconnect` function or
event handler. Consequently, there are some possible sequences of
events. We must check events twice, because without getting an exited
event, `exit_status` will be None. But, we don't know the order of
events (for example, we can get terminated event before exited event),
so we check events by filter. It is correct, because terminated event
will be sent only once (guarded by `llvm::call_once`).
This patch moved from
[145010](https://github.com/llvm/llvm-project/pull/145010) and based on
idea from this
[comment](https://github.com/llvm/llvm-project/pull/145010#discussion_r2159637210).
.. from the guts of GDBRemoteCommunication to ~top level.
This is motivated by #131519 and by the fact that's impossible to guess
whether the author of a symlink intended it to be a "convenience
shortcut" -- meaning it should be resolved before looking for related
files; or an "implementation detail" -- meaning the related files should
be located near the symlink itself.
This debate is particularly ridiculous when it comes to lldb-server
running in platform mode, because it also functions as a debug server,
so what we really just need to do is to pass /proc/self/exe in a
platform-independent manner.
Moving the location logic higher up achieves that as lldb-platform (on
non-macos) can pass `HostInfo::GetProgramFileSpec`, while liblldb can
use the existing complex logic (which only worked on liblldb anyway as
lldb-platform doesn't have a lldb_private::Platform instance).
Another benefit of this patch is a reduction in dependency from
GDBRemoteCommunication to the rest of liblldb (achieved by avoiding the
Platform dependency).
The
[protocol](https://microsoft.github.io/debug-adapter-protocol//specification.html#Types_Source)
expects that `sourceReference` be less than `(2^31)-1`, but we currently
represent memory address as source reference, this can be truncated
either when sending through json or by the client. Instead, generate new
source references based on the memory address.
Make the `ResolveSource` function return an optional source.
# Benefit
This patch fixes:
1. After `platform select ios-simulator`, `platform process list` will
now print processes which are running in the iOS simulator. Previously,
no process will be listed.
2. After `platform select ios-simulator`, `platform attach --name
<name>` will succeed. Previously, it will error out saying no process is
found.
# Several bugs that is being fixed
1. During the process listing, add `aarch64` to the list of CPU types
for which iOS simulators are checked for.
2. Given a candidate process, when checking for simulators, the original
code will find the desired environment variable (`SIMULATOR_UDID`) and
set the OS to iOS, but then the immediate next environment variable will
set it back to macOS.
3. For processes running on simulator, set the triple's `Environment` to
`Simulator`, so that such processes can pass the filtering [in this
line](https://fburl.com/8nivnrjx). The original code leave it as the
default `UnknownEnvironment`.
# Manual test
**With this patch:**
```
royshi-mac-home ~/public_llvm/build % bin/lldb
(lldb) platform select ios-simulator
(lldb) platform process list
240 matching processes were found on "ios-simulator"
PID PARENT USER TRIPLE NAME
====== ====== ========== ============================== ============================
40511 28844 royshi arm64-apple-ios-simulator FocusPlayground // my toy iOS app running on simulator
... // omit
28844 1 royshi arm64-apple-ios-simulator launchd_sim
(lldb) process attach --name FocusPlayground
Process 40511 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
frame #0: 0x0000000104e3cb70 libsystem_kernel.dylib`mach_msg2_trap + 8
libsystem_kernel.dylib`mach_msg2_trap:
-> 0x104e3cb70 <+8>: ret
... // omit
```
**Without this patch:**
```
$ bin/lldb
(lldb) platform select ios-simulator
(lldb) platform process list
error: no processes were found on the "ios-simulator" platform
(lldb) process attach --name FocusPlayground
error: attach failed: could not find a process named FocusPlayground
```
# Unittest
See PR.
```
Traceback (most recent call last):
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py", line 8, in <module>
lldbsuite.test.run_suite()
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py", line 1064, in run_suite
visit("Test", dirpath, filenames)
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py", line 706, in visit
visit_file(dir, name)
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py", line 649, in visit_file
module = __import__(base)
^^^^^^^^^^^^^^^^
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py", line 14, in <module>
class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase):
File "/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py", line 29, in TestDAP_runInTerminal
@skipIfAsan
^^^^^^^^^^
NameError: name 'skipIfAsan' is not defined
```
These fail with on the macOS ASAN CI with following error:
```
07:10:31 ======================================================================
07:10:31 FAIL: test_runInTerminalWithObjectEnv (TestDAP_runInTerminal.TestDAP_runInTerminal)
07:10:31 Tests the "runInTerminal" reverse request. It makes sure that the IDE can
07:10:31 ----------------------------------------------------------------------
07:10:31 Traceback (most recent call last):
07:10:31 File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 149, in wrapper
07:10:31 return func(*args, **kwargs)
07:10:31 File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py", line 83, in test_runInTerminalWithObjectEnv
07:10:31 self.build_and_launch(program, runInTerminal=True, env={"FOO": "BAR"})
07:10:31 File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 488, in build_and_launch
07:10:31 return self.launch(program, **kwargs)
07:10:31 File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 470, in launch
07:10:31 self.assertTrue(
07:10:31 AssertionError: False is not true : launch failed (process exited during launch or attach)
07:10:31 Config=arm64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/bin/clang
```
These fail ever since they were enabled in https://github.com/llvm/llvm-project/pull/144954.
Skip these again when testing with ASAN.
It is failing on our Windows on Arm bot:
AssertionError: False is not true : launch failed (Unimplemented)
Given that all the others are skipped on Windows, I assume this
failure is expected too.
Replace `isTestSupported` function with `skipIfBuildType` annotation.
Test that uses the `IsTestSupported` function are no longer run, as the
size of lldb-dap binary is now more than `1mb`.
Update the broken test.
Fixes#108621
We could probably check if the test now passes on `linux arm` since it
was disabled because it timed out. I experienced the timeout after
replacing the `IsTestSupported` with `skipIfBuildType`.
In DebugCommunication, we currently are using 2 thread to drive
lldb-dap. At the moment, they make an attempt at only synchronizing the
`recv_packets` between the reader thread and the main test thread. Other
stateful properties of the debug session are not guarded by a
locks/mutex.
To mitigate this, I am moving any state updates to the main thread
inside the `_recv_packet` method to ensure that between calls to
`_recv_packet` the state does not change out from under us in a test.
This does mean the precise timing of events has changed slightly as a
result and I've updated the existing tests that fail for me locally with
this new behavior.
I think this should result in overall more predictable behavior, even if
the test is slow due to the host workload or architecture differences.
---------
Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>
Currently when jitting expressions, LLDB scans the IR instructions of
the `$__lldb_expr` and will insert a call to a utility function for each
load/store instruction. The purpose of the utility funciton is to
dereference the load/store operand. If that operand was an invalid
pointer the utility function would trap and LLDB asks the IR checker
whether it was responsible for the trap, in which case it prints out an
error message saying the expression dereferenced an invalid pointer.
This is a lot of setup for not much gain. In fact, creating/running this
utility expression shows up as ~2% of the expression evaluation time
(though we cache them for subsequent expressions). And the error message
we get out of it is arguably less useful than if we hadn't instrumented
the IR. It was also untested.
Before:
```
(lldb) expr int a = *returns_invalid_ptr()
error: Execution was interrupted, reason: Attempted to dereference an invalid pointer..
The process has been returned to the state before expression evaluation.
```
After:
```
(lldb) expr int a = *returns_invalid_ptr()
error: Expression execution was interrupted: EXC_BAD_ACCESS (code=1, address=0x5).
The process has been returned to the state before expression evaluation.
```
This patch removes this IR checker.
This adds new types for setExceptionBreakpoints and adds support for
`supportsExceptionFilterOptions`, which allows exception breakpoints to
set a condition.
While testing this, I noticed that obj-c exception catch breakpoints may
not be working correctly in lldb-dap.
This adds a new 'CapabilitiesEventBody' type for having a well
structured type for the event and updates the 'restart' request
to dynamically set their capabilities.
This reverts commit 4b6c608615a285d81132acf8e33b81b2ec2c9bf9 and
follow-up commits 159de3633640a5cb2d322ebe8cc4ec0c1c9a896d and
c9e1c52e2e75a91a44a98df818cc9bd11655e51d because this breaks
TestDAP_stepInTargets.py on Darwin.
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake
Trimming unused imports, adjusting the test to use the `DEFAULT_TIMEOUT`
instead of a custom timeout and adjusting the flow to stopOnEntry for
improving consistency.
Previously, we incorrectly handled the disconnect operation if we signal
lldb-dap running in server mode.
Updated to correctly disconnect the attach vs launch behavior on server
shutdown.
Attempt to improve tests by synchronously waiting for breakpoints to
resolve. Not sure if it will fix all the tests but I think it should
make the tests more stable
This reverts commit 8a49db35f45e56c92522c6079e51553e80c07aec.
Due to failures on Arm and AArch64 Linux:
https://lab.llvm.org/buildbot/#/builders/59/builds/18540https://lab.llvm.org/buildbot/#/builders/18/builds/16759
File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py", line 22, in assertEvaluateFailure
self.assertNotIn(
AssertionError: 'result' unexpectedly found in {'memoryReference': '0xFFFFF7CB3060', 'result': '0x0000000000000000', 'type': 'int *', 'variablesReference': 7}
FAIL: test_generic_evaluate_expressions (TestDAP_evaluate.TestDAP_evaluate)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py", line 228, in test_generic_evaluate_expressions
self.run_test_evaluate_expressions(enableAutoVariableSummaries=False)
File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py", line 117, in run_test_evaluate_expressions
self.assertEvaluateFailure("list") # local variable of a_function
File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py", line 22, in assertEvaluateFailure
self.assertNotIn(
AssertionError: 'result' unexpectedly found in {'memoryReference': '0xFFFFF7CB3060', 'result': '0x0000000000000000', 'type': 'int *', 'variablesReference': 7}
Config=aarch64-/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang
The second one is because our bots have the libc debug info package installed,
the first, no idea.
When a target thread returned an empty but not `nullptr` string as its
name, the thread would show up with an empty name in lldb-dap.
I don't know how this works on macOS and Linux, but on Windows,
[`TargetThreadWindows::GetName`](deedc8a181/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp (L178-L204))
returns a non-null pointer to an empty string, because on MSVC's STL,
`std::string{}.c_str()` returns a pointer to inside the object (the SSO
storage).
This changes the check in `CreateThread`, when no custom thread
formatter is set, to check for the length of the thread and queue name
instead of it being `nullptr`.
Improving the readability and correctness of DebugCommunication by
adding type annotations to many parts of the library and trying to
improve the implementation of a few key areas of the code to better
handle correctness.
Specifically, this refactored the
`DebugCommunication._handle_recv_packet` function to ensure consistency
with the reader thread when handling state changes and improved the
`DebugCommunication._recv_packet` helper to make it easier to follow by
adding some additional helpers.
The architectures provided to skipIf / expectedFail are regular
expressions (v. _match_decorator_property() in decorators.py
so on Darwin systems "arm64" would match the skips for "arm" (32-bit
Linux). Update these to "arm$" to prevent this, and also update
three tests (TestBuiltinFormats.py, TestCrossDSOTailCalls.py,
TestCrossObjectTailCalls.py) that were skipped for arm64 via this
behavior, and need to be skipped or they will fail.
This was moviated by the new TestDynamicValue.py test which has
an expected-fail for arm, but the test was passing on arm64 Darwin
resulting in failure for the CIs.
- `CreateRunInTerminalReverseRequest` is passed a `StringMap` by value,
whose keys are owned and deallocated after return.
- Target args are wrongly specified as reverse request (launcher) args.
- Test case error message did not catch up with a0aa5f8.
All runInTerminal tests are passing with this applied.
New test added by https://github.com/llvm/llvm-project/pull/139969.
On Windows we need debug information to be able to break on the
function (we don't need it for main, but I assume that's a special case).
So disable the test on Windows.
I tried a few tricks like making a global label in assembly, but
that doesn't show up without debug info either.
