If this test fails, you're likely going to see something like "Assertion
Error: A != B" which doesn't really give much explanation for why this
failed.
Instead of ignoring the error, we should assert that it succeeded. This
will lead to a better error message, for example:
`AssertionError: 'memory write failed for 0x102d7c018' is not success`
The test `SocketTest::TCPListen0MultiListenerGetListeningConnectionURI`
is failing on hosts that do not map `localhost` to both an ipv4 and ipv6
address. For example this build
https://lab.llvm.org/buildbot/#/builders/195/builds/1909.
To fix this, I added a helper to validate if the host has an /etc/hosts
entry for both ipv4 and ipv6, otherwise we skip the test.
Indexing a single DWARF unit is a fairly small task, which means the
overhead of enqueueing a task for each unit is not negligible (mainly
because introduces a lot of synchronization points for queue management,
memory allocation etc.). This is particularly true if the binary was
built with type units, as these are usually very small.
This essentially brings us back to the state before
https://reviews.llvm.org/D78337, but the new implementation is built on
the llvm ThreadPool, and I've added a small improvement -- we now
construct one "index set" per thread instead of one per unit, which
should lower the memory usage (fewer small allocations) and make the
subsequent merge step faster.
On its own this patch doesn't actually change the performance
characteristics because we still have one choke point -- progress
reporting. I'm leaving that for a separate patch, but I've tried that
simply removing the progress reporting gives us about a 30-60% speed
boost.
Currently, the link to the issue tracker takes you to the Github source
repository, rather than the Github issue tracker. This fixes the link
and includes the lldb-dap label in both the issue and PR URL.
LLDB can crash in TypeSystemClang::GetIndexOfChildMemberWithName, at a
point where it pushes an index onto the child_indexes vector, tries to
call itself recursively, then tries to pop the entry from child_indexes.
The problem is that the recursive call can clear child_indexes, so that
this code ends up trying to pop an already empty vector. This change
saves the old vector before the push, then restores the saved vector
rather than trying to pop.
Support finding the lldb-dap binary with `xcrun` on Darwin or in PATH on
all other platforms.
Unfortunately, this PR is larger than I would like because it removes
the `lldbDapOptions`. I believe these options are not necessary, and as
previously implemented, they caused a spurious warning with this change.
The problem was that the options were created before the custom factory.
By moving the creation logic into the factory, we make sure it's only
called after the factory has been registered. The upside is that this
simplifies the code and removes a level of indirection.
On systems supporting ting ipv4 and ipv6 the second socket to initialize
will not update the listening address correctly after the call to `bind`.
This results in the second address listed in
`Socket::GetListeningConnectionURI` to have port `:0`, which is
incorrect.
To fix this, correct which address is used to detect the port and update
the unit tests to cover this use case.
Additionally, I updated the SocketTest's to only parameterize tests that
can work on ipv4 or ipv6. This means tests like
`SocketTest::DecodeHostAndPort` are only run once, instead of twice
since they do not change behavior based on parameters.
I also included a new unit test to cover listening on `localhost:0`,
validating both sockets correctly list the updated port.
Update the error returns in ValueObject::CastToBasicType and
ValueObject::CastToEnumType to create new errors and return a
ValueObjectConstResult with the error, rather tnan updating the error in
(and returning) the input ValueObject.
Specify ENABLE_THREADS := YES within test's Makefile instead of passing
-lpthread explicitly via the compiler's CFLAGS options.
Refactoring fix.
Co-authored-by: Vladimir Vereschaka <vvereschaka@accesssoftek.com>
The markdown tables in the README aren't getting rendered correctly on
the LLDB-DAP page in the Visual Studio arketplace [1]. This is a
somewhat speculative fix as the table itself appears to be correct. Even
if this change doesn't fix it, the new formatting significantly improves
the readability.
[1] https://marketplace.visualstudio.com/items?itemName=llvm-vs-code-extensions.lldb-dap
The command was using the wrong publisher name ("llvm" rather than
"llvm-vs-code-extensions") resulting in the command complaining:
```
npm run vscode-uninstall
> lldb-dap@0.2.6 vscode-uninstall
> code --uninstall-extension llvm.lldb-dap
Extension 'llvm.lldb-dap' is not installed.
Make sure you use the full extension ID, including the publisher, e.g.: ms-dotnettools.csharp
```
This PR fixes a simple SWIG issue with SBMemoryRegionInfoList not being
iterable out-of-the-box. This is mostly because of limitations to the
`lldb_iter` function, which doesn't allow for specifying arguments to
the size / iter functions passed.
Before:
```
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> for region in lldb.process.GetMemoryRegions():
... print(region)
...
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/opt/llvm/stable/Toolchains/llvm-sand.xctoolchain/usr/lib/python3.10/site-packages/lldb/__init__.py", line 114, in lldb_iter
yield elem(i)
TypeError: SBMemoryRegionInfoList.GetMemoryRegionAtIndex() missing 1 required positional argument: 'region_info'
```
After:
```
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> for region in lldb.process.GetMemoryRegions():
... print(region)
...
[0x0000000000200000-0x00000000002cf000 R--]
[0x00000000002cf000-0x0000000000597000 R-X]
[0x0000000000597000-0x00000000005ad000 R--]
[0x00000000005ad000-0x00000000005b1000 RW-]
[0x00000000005b1000-0x0000000000b68000 RW-]
[0x000000007fff7000-0x000000008fff7000 RW-]
[0x000002008fff7000-0x000010007fff8000 RW-]
[0x0000503000000000-0x0000503000010000 RW-]
[0x0000503e00000000-0x0000503e00010000 RW-]
[0x0000504000000000-0x0000504000010000 RW-]
[0x0000504e00000000-0x0000504e00010000 RW-]
[0x000050d000000000-0x000050d000010000 RW-]
[0x000050de00000000-0x000050de00010000 RW-]
[0x000050e000000000-0x000050e000010000 RW-]
[0x000050ee00000000-0x000050ee00010000 RW-]
[0x0000511000000000-0x0000511000010000 RW-]
[0x0000511e00000000-0x0000511e00010000 RW-]
[0x0000513000000000-0x0000513000010000 RW-]
...
```
The ManualDWARFIndex class can create a index cache if the LLDB index
cache is enabled. This used to save the index to the same file,
regardless of wether the cache was a full index (no .debug_names) or a
partial index (have .debug_names, but not all .o files were had
.debug_names). So we could end up saving an index cache that was
partial, and then later load that partial index as if it were a full
index if the user set the 'settings set
plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us
to ignore the .debug_names section, and if the index cache was enabled,
we could end up loading the partial index as if it were a full DWARF
index.
This patch detects when the ManualDWARFIndex is being used with
.debug_names, and saves out a cache file with a suffix of "-full" or
"-partial" to avoid this issue.
.. in the global namespace
The problem was the interaction of #116989 with an optimization in
GetTypesWithQuery. The optimization was only correct for non-exact
matches, but that didn't matter before this PR due to the "second layer
of defense". After that was removed, the query started returning more
types than it should.
This reverts commit 2526d5b168, reapplying
ba14dac481 after fixing the conflict with
#117532. The change is that Function::GetAddressRanges now recomputes
the returned value instead of returning the member. This means it now
returns a value instead of a reference type.
This is a follow-up/reimplementation of #115730. While working on that
patch, I did not realize that the correct (discontinuous) set of ranges
is already stored in the block representing the whole function. The
catch -- ranges for this block are only set later, when parsing all of
the blocks of the function.
This patch changes that by populating the function block ranges eagerly
-- from within the Function constructor. This also necessitates a
corresponding change in all of the symbol files -- so that they stop
populating the ranges of that block. This allows us to avoid some
unnecessary work (not parsing the function DW_AT_ranges twice) and also
results in some simplification of the parsing code.
SBFunction::GetEndAddress doesn't really make sense for discontinuous
functions, so I'm declaring it deprecated. GetStartAddress sort of makes
sense, if one uses it to find the functions entry point, so I'm keeping
that undeprecated.
I've made the test a Shell tests because these make it easier to create
discontinuous functions regardless of the host os and architecture. They
do make testing the python API harder, but I think I've managed to come
up with something not entirely unreasonable.
`frame variable` supports nested variable access, which the API calls "variable
expression paths". This change updates `dwim-print` to support a subset of supported
variable expression paths.
Consider the expression `a->b`. In C++, the arrow operator can be overloaded, and where
that is the case, expression evaluation must be used to evaluate it, not frame variable.
Likewise, the subscript operator can be overloaded.
To avoid those cases, this change introduces a limited support for variable expression
paths. Use of the dot operator is allowed.
Additionally, this change allows `dwim-print` to directly access children of `this` and
`self` (see AllowDirectIVarAccess). This functionality is also provided by the same
`GetValueForVariableExpressionPath` method.
rdar://104348908
Reduced MAX_ATTEMPTS to 10.
The sleep time will be 3, 3.6, 4.3, ..., 13, 15.
The total sleep time is 78 + 0.5 * MAX_CONNECT_ATTEMPTS * MAX_ATTEMPTS = 128.
Note the timeout is 600, so the rest time must be enough.
Increased the port range (12000..20000).
During lldb testing dotest.py opens files to dump testcase results, but
doesn't close them.
This patch makes necessary changes to fix the file descriptors leak.
Fixes warning: 'sprintf' is deprecated: This function is provided for
compatibility reasons only. Due to security concerns inherent in the
design of sprintf(3), it is highly recommended that you use snprintf(3)
instead.
The Mach-O load commands have an LC_SYMTAB / struct symtab_command which
represents the offset of the symbol table (nlist records) and string
table for this binary. In a mach-o binary on disk, these are file
offsets. If a mach-o binary is loaded in memory with all segments
consecutive, the `symoff` and `stroff` are the offsets from the TEXT
segment (aka the mach-o header) virtual address to the virtual address
of the start of these tables.
However, if a Mach-O binary is a part of the shared cache, then the
segments will be separated -- they will have different slide values. And
it is possible for the LINKEDIT segment to be greater than 4GB away from
the TEXT segment in the virtual address space, so these 32-bit offsets
cannot express the offset from TEXT segment to these tables.
Create separate uint64_t variables to track the offset to the symbol
table and string table, instead of reusing the 32-bit ones in the
symtab_command structure.
rdar://140432279
It's never set to true. Also, using inheritable FDs in a multithreaded
process pretty much guarantees descriptor leaks. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.
Clean up some unnecessary include paths. The use of `LibXml2::LibXml2`
with `target_link_libraries` on `libLLDBHost` ensures that the header
search path is properly propagated.
It's basically true already (except for a brief time during
construction). This patch makes sure the objects are constructed with a
valid parent and enforces this in the type system, which allows us to
get rid of some nullptr checks.
Allows us to stop waiting for a connection if it doesn't come in a
certain amount of time. Right now, I'm keeping the status quo (infitnite
wait) in the "production" code, but using smaller (finite) values in
tests. (A lot of these tests create "loopback" connections, where a
really short wait is sufficient: on linux at least even a poll (0s wait)
is sufficient if the other end has connect()ed already, but this doesn't
seem to be the case on Windows, so I'm using a 1s wait in these cases).
On #110065 the changes to LinuxSigInfo Struct introduced some variables
that will differ in size on 32b or 64b. I've rectified this by setting
them all to build independent types.
In case of an error GetBlock would return a reference to a Block without
adding it to a parent. This doesn't seem like a good idea, and none of
the other plugins do that.
This patch fixes that by propagating errors (well, null pointers...) up
the stack.
I don't know of any specific problem that this solves, but given that
this occurs only when something goes very wrong (e.g. a corrupted PDB
file), it's quite possible noone has run into this situation, so we
can't say the code is correct either. It also gets in the way of a
refactor I'm contemplating.
In #108907, the index classes started filtering the DIEs according to
the full type query (instead of just the base name). This means that the
checks in SymbolFileDWARF are now redundant.
I've also moved the non-redundant checks so that now all checking is
done in the DWARFIndex class and the caller can expect to get the final
filtered list of types.
ELF core debugging fix#117070 broke TestLoadUnload.py tests due to
GetModuleSpec call, ProcessGDBRemote fetches modules from remote. Revise
the original PR, renamed FindBuildId to FindModuleUUID.
Making a breakpoint on a line causes an error on aarch64-pc-windows.
This patch changes the test so that a breakpoint can be made on a
function name.
#117168
The problem here is the assumption that the entire function will be
placed in a single section. This will ~never be the case for a
discontinuous function, as the point of splitting the function is to let
the linker group parts of the function according to their "hotness".
The fix is to change the offset computation to use file addresses
instead.