Completion requests have two fields that are essentially unimplemented:
`m_match_start_point` and `m_max_return_elements`. This would've been
okay, if it wasn't for the fact that this caused a bunch of useless
parameters to be passed around. Occasionally there would be a comment or
assert saying that they are not supported. This patch removes them.
llvm-svn: 367385
When investigating a completion bug I got confused by the API.
LongestCommonPrefix finds the longest common prefix of the strings in
the string list. Instead of returning that string through an output
argument, just return it by value.
llvm-svn: 367384
SymbolFilePDB tests were using GetTypeSystemForLanguage but weren't
changed to accomodate the use of an llvm::Expected. I adjusted them
accordingly.
llvm-svn: 367368
Summary:
Instead of having SymbolVendor coordinate Symtab construction between
Symbol and Object files, make the SymbolVendor function a passthrough,
and put all of the logic into the SymbolFile.
Reviewers: clayborg, JDevlieghere, jingham, espindola
Subscribers: emaste, mgorny, arichardson, MaskRay, lldb-commits
Differential Revision: https://reviews.llvm.org/D65208
llvm-svn: 367086
The file collector class is useful for creating reproducers,
not just for LLDB, but for other tools as well in LLVM/Clang.
Differential Revision: https://reviews.llvm.org/D65237
llvm-svn: 366956
This patch replaces explicit calls to log::Printf with the new LLDB_LOGF
macro. The macro is similar to LLDB_LOG but supports printf-style format
strings, instead of formatv-style format strings.
So instead of writing:
if (log)
log->Printf("%s\n", str);
You'd write:
LLDB_LOG(log, "%s\n", str);
This change was done mechanically with the command below. I replaced the
spurious if-checks with vim, since I know how to do multi-line
replacements with it.
find . -type f -name '*.cpp' -exec \
sed -i '' -E 's/log->Printf\(/LLDB_LOGF\(log, /g' "{}" +
Differential revision: https://reviews.llvm.org/D65128
llvm-svn: 366936
Summary: Make debugserver a tool like lldb-server, so it can be included/excluded via `LLDB_TOOL_DEBUGSERVER_BUILD`. This replaces the old `LLDB_NO_DEBUGSERVER` flag. Doing the same for darwin-debug while I am here.
Reviewers: xiaobai, JDevlieghere, davide
Reviewed By: xiaobai, JDevlieghere
Subscribers: mgorny, lldb-commits, #lldb
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D64994
llvm-svn: 366631
Summary:
We can always build debugserver, but we can't always sign it to be useable for testing. `LLDB_USE_SYSTEM_DEBUGSERVER` should only tell whether or not the system debugserver should be used for testing.
The old behavior complicated the logic around debugserver a lot. The new logic sorts out most of it.
Please note that this patch is in early stage and needs some more testing. It should not affect platfroms other than Darwin. It builds on Davide's approach to validate the code-signing identity at configuration time.
What do you think?
Reviewers: xiaobai, JDevlieghere, davide, compnerd, friss, labath, mgorny, jasonmolenda
Reviewed By: JDevlieghere
Subscribers: lldb-commits, #lldb
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D64806
llvm-svn: 366433
Summary:
This is the fifth patch to improve module loading in a series that started here (where I explain the motivation and solution): D62499
Reading strings with ReadMemory is really slow when reading the path of the shared library. This is because we don't know the length of the path so use PATH_MAX (4096) and these strings are actually super close to the boundary of an unreadable page. So even though we use process_vm_readv it will usually fail because the read size spans to the unreadable page and we then default to read the string word by word with ptrace.
This new function is very similar to another ReadCStringFromMemory that already exists in lldb that makes sure it never reads cross page boundaries and checks if we already read the entire string by finding '\0'.
I was able to reduce the GetLoadedSharedLibraries call from 30ms to 4ms (or something of that order).
Reviewers: clayborg, xiaobai, labath
Reviewed By: labath
Subscribers: emaste, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62503
llvm-svn: 363750
Before this patch, reproducers weren't relocatable. The reproducer
contained hard coded paths in the VFS mapping, as well in the yaml file
listing the different input files for the command interpreter. This
patch changes that:
- Use relative paths for the DataCollector.
- Use an overlay prefix for the FileCollector.
Differential revision: https://reviews.llvm.org/D63467
llvm-svn: 363697
Summary:
This is the third patch to improve module loading in a series that started here (where I explain the motivation and solution): D62499
Add functions to read the r_debug location to know where the linked list of loaded libraries are so I can generate the `xfer:libraries-svr4` packet.
I'm also using this function to implement `GetSharedLibraryInfoAddress` that was "not implemented" for linux.
Most of this code was inspired by the current ds2 implementation here: https://github.com/facebook/ds2/blob/master/Sources/Target/POSIX/ELFProcess.cpp.
Reviewers: clayborg, xiaobai, labath
Reviewed By: clayborg, labath
Subscribers: emaste, krytarowski, mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62501
llvm-svn: 363458
This test seems to occasionally fail because editline returns a
different number of lines. Rewrite the message in such a way that we
also see the actual lines when that happens (and not just their count).
Also, clean up the dependencies of the test while I'm in there.
llvm-svn: 363404
Summary:
The motivation for this was me wanting to make the validity of dwarf
DIERefs explicit (via llvm::Optional<DIERef>). This meant that the class
would no longer have a default constructor. As the DIERef was being
stored in a UniqueCStringMap, this meant that this container (like all
standard containers) needed to work with non-default-constructible types
too.
This part is achieved by removing the default constructors for the map
entry types, and providing appropriate comparison overloads so that we
can search for map entries without constructing a dummy entry. While
doing that, I took the opportunity to modernize the code, and add some
tests. Functions that were completely unused are deleted.
This required also some changes in the Symtab code, as it was default
constructing map entries, which was not impossible even though its
value type was default-constructible. Technically, these changes could
be avoided with some SFINAE on the entry type, but I felt that the code
is cleaner this way anyway.
Reviewers: JDevlieghere, sgraenitz
Subscribers: mgorny, aprantl, lldb-commits
Differential Revision: https://reviews.llvm.org/D63268
llvm-svn: 363357
This replaces the `info` typedef with a nested struct named Info. This
means we now have FooProvider and FooProvider::Info, instead of two
related but separate classes FooProvider and FooInfo. This change is
mostly cosmetic.
llvm-svn: 363211
Summary:
This is the first of a few patches I have to improve the performance of dynamic module loading on Android.
In this first diff I'll describe the context of my main motivation and will then link to it in the other diffs to avoid repeating myself.
## Motivation
I have a few scenarios where opening a specific feature on an Android app takes around 40s when lldb is attached to it. The reason for that is because 40 modules are dynamicly loaded at that point in time and each one of them is taking ~1s.
## The problem
To learn about new modules we have a breakpoint on a linker function that is called twice whenever a module is loaded. One time just before it's loaded (so lldb can check which modules are loaded) and another right after it's loaded (so lldb can check again which ones are loaded and calculate the diference).
It's figuring out which modules are loaded that is taking quite some time. This is currently done by traversing the linked list of loaded shared libraries that the linker maintains in memory. Each item in the linked list requires its own `x` packet sent to the gdb server (this is android so the network also plays a part). In my scenario there are 400+ loaded libraries and even though we read 0x800 worth of bytes at a time we still make ~180 requests that end up taking 150-200ms.
We also do this twice, once before the module is loaded (state = eAdd) and another right after (state = eConsistent) which easly adds up to ~400ms per module.
## A solution
**Implement `xfer:libraries-svr4` in lldb-server:**
I noticed in the code that loads the new modules that it had support for the `xfer:libraries-svr4` packet (added ~4 years ago to support the ds2 debug server) but we didn't support it in lldb-server. This single packet returns an xml list of all the loaded modules by the process. The advantage is that there's no more need to make 180 requests to read the linked list. Additionally this new requests takes around 10ms.
**More efficient usage of the `xfer:libraries-svr4` packet in lldb:**
When `xfer:libraries-svr4` is available the Process class has a `LoadModules` function that requests this packet and then loads or unloads modules based on the current list of loaded modules by the process.
This is the function that is used by the DYLDRendezvous class to get the list of loaded modules before and after the module is loaded. However, this is really not needed since the LoadModules function already loaded or unloaded the modules accordingly. I changed this strategy to call LoadModules only once (after the process has loaded the module).
**Bugs**
I found a few issues in lldb while implementing this and have submitted independent patches for them.
I tried to devide this into multiple logical patches to make it easier to review and discuss.
## Tests
I wanted to put these set of diffs up before having all the tests up and running to start having them reviewed from a techical point of view. I'm also having some trouble making the tests running on linux so I need more time to make that happen.
# This diff
The `xfer` packages follow the same protocol, they are requested with `xfer:<object>:<read|write>:<annex>:<offset,length>` and a return that starts with `l` or `m` depending if the offset and length covers the entire data or not. Before implementing the `xfer:libraries-svr4` I refactored the `xfer:auxv` to generically handle xfer packets so we can easly add new ones.
The overall structure of the function ends up being:
* Parse the packet into its components: object, offset etc.
* Depending on the object do its own logic to generate the data.
* Return the data based on its size, the requested offset and length.
Reviewers: clayborg, xiaobai, labath
Reviewed By: labath
Subscribers: mgorny, krytarowski, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62499
llvm-svn: 362982
The x86 assembly inspection engine has code to support detecting a
mid-function epilogue that ends in a RET instruction; add support for
recognizing an epilogue that ends in a JMP, and add a check that the
unwind state has been restored to the original stack setup; reinstate
the post-prologue unwind state after this JMP instruction.
The assembly inspection engine used for other architectures,
UnwindAssemblyInstEmulation, detects mid-function epilogues by
tracking branch instructions within the function and "forwards"
the current unwind state to the targets of the branches. If
an epilogue unwinds the stack and exits, followed by a branch
target, we get back to the correct unwind state. The x86
unwinder should move to this same algorithm, or possibly even
look at implementing an x86 instruction emulation plugin and
get UnwindAssemblyInstEmulation to work for x86 too. I added
a branch instruction recognizier method that will be necessary
if we want to switch the algorithm.
Differential Revision: https://reviews.llvm.org/D62764
<rdar://problem/51074422>
llvm-svn: 362456
Summary:
This test base class is missing the teardown making the second set of tests extending it to fail in an assertion in the FileSystem::Initialize() (as it's being initialized twice).
Not sure why this isn't failing the build bots.. (unless they're running without asserts?).
With this fix `ninja LLDBServerTests && ./tools/lldb/unittests/tools/lldb-server/tests/LLDBServerTests` successfully runs and passes all tests.
Reviewers: clayborg, xiaobai, labath
Reviewed By: xiaobai, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62788
llvm-svn: 362406
There is likely also an underlying bug in all code that calls
CPlusPlusNameParser with nullptrs, but this patch can also stand for
itself.
rdar://problem/49072829
llvm-svn: 362177
Summary:
My main goal here is to make lldb-server work with Android Studio.
This is currently not the case because lldb-server is started in platform mode listening on a domain socket. When Android Studio connects to it lldb-server crashes because even though it's listening on a domain socket as soon as it gets a connection it asserts that it's a TCP connection, which will obviously fails for any non-tcp connection.
To do this I came up with a new method called GetConnectURI() in Socket that returns the URI needed to connect to the connected portion of the socket.
Reviewers: labath, clayborg, xiaobai
Reviewed By: labath
Subscribers: mgorny, jfb, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62089
llvm-svn: 362173
Summary:
This is useless and it's giving warnings in the build bots:
/home/motus/netbsd8/netbsd8/llvm/tools/lldb/unittests/Utility/TimerTest.cpp:67:43: warning: use of assignment suppression and length modifier together in gnu_scanf format [-Wformat=]
Reviewers: xiaobai
Subscribers: krytarowski, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62626
llvm-svn: 362107
The issue was caused by the error checking code that was added. It was incorrectly adding an extra abbreviation when DWARFEnumState::Complete was received since it would push an extra abbreviation onto the list with the abbreviation code of zero. This cause m_idx_offset in each DWARFAbbreviationDeclarationSet to be set to UINT32_MAX. This valid indicates we must linearly search for attributes, not access them in O(1) time. This caused every DWARFDebugInfoEntry that would try to get its DWARFAbbreviationDeclaration from the CU's DWARFAbbreviationDeclarationSet to always linearly search the abbreviation set for a given abbreviation code. Easy to see why this would cause things to be slow.
This regression was caused by: https://reviews.llvm.org/D59370. I asked to ensure there was no regression is parsing or access speed, but that must not have been done. In my test with 40 DWARF files trying to set a breakpoint by function name and in a header file, I see a 8% speed improvement with this fix.
There was no regression in correctness, just very inefficient access.
Added full unit testing for DWARFAbbreviationDeclarationSet parsing to ensure this doesn't regress.
Differential Revision: https://reviews.llvm.org/D62630
llvm-svn: 362105
Summary:
The `log timer dump` is showing the time of the function itself minus any function that is called from this one that also happens to be timed. However, this is really not obvious and it also makes it hard to understand the time spent in total and also which children are actually taking the time.
To get a better reading of the timer dump I added the total, children (which I named child) and also the hit count. I used these timers to figure out a performance issue and only after adding this things were more clear to me.
It looks like this:
```
(lldb) log timer dump
35.447713617 sec (total: 35.449s; child: 0.001s; count: 1374) for void SymbolFileDWARF::Index()
29.717921481 sec (total: 29.718s; child: 0.000s; count: 8230500) for const lldb_private::ConstString &lldb_private::Mangled::GetDemangledName(lldb::LanguageType) const
21.049508865 sec (total: 24.683s; child: 3.633s; count: 1399) for void lldb_private::Symtab::InitNameIndexes()
...
```
Reviewers: clayborg, teemperor, labath, espindola, xiaobai
Reviewed By: labath, xiaobai
Subscribers: emaste, mgorny, arichardson, eraman, MaskRay, jdoerfert, labath, davide, teemperor, aprantl, erik.pilkington, jfb, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61235
llvm-svn: 361987
Summary:
This is a general fix for the ConnectionFileDescriptor class but my main motivation was to make lldb-server working with IPv6.
The connect URI can use square brackets ([]) to wrap the interface part of the URI (e.g.: <scheme>://[<interface>]:<port>). For IPv6 addresses this is a must since its ip can include colons and it will overlap with the port colon otherwise. The URIParser class parses the square brackets correctly but the ConnectionFileDescriptor doesn't generate them for IPv6 addresses making it impossible to connect to the gdb server when using this protocol.
How to reproduce the issue:
```
$ lldb-server p --server --listen [::1]:8080
...
$ lldb
(lldb) platform select remote-macosx
(lldb) platform connect connect://[::1]:8080
(lldb) platform process -p <pid>
error: unable to launch a GDB server on 'computer'
```
The server was actually launched we were just not able to connect to it. With this fix lldb will correctly connect. I fixed this by wrapping the ip portion with [].
Reviewers: labath
Reviewed By: labath
Subscribers: xiaobai, mgorny, jfb, lldb-commits, labath
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61833
llvm-svn: 361898
Summary:
NFC = [[ https://llvm.org/docs/Lexicon.html#nfc | Non functional change ]]
This commit is the result of modernizing the LLDB codebase by using
`nullptr` instread of `0` or `NULL`. See
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html
for more information.
This is the command I ran and I to fix and format the code base:
```
run-clang-tidy.py \
-header-filter='.*' \
-checks='-*,modernize-use-nullptr' \
-fix ~/dev/llvm-project/lldb/.* \
-format \
-style LLVM \
-p ~/llvm-builds/debug-ninja-gcc
```
NOTE: There were also changes to `llvm/utils/unittest` but I did not
include them because I felt that maybe this library shall be updated in
isolation somehow.
NOTE: I know this is a rather large commit but it is a nobrainer in most
parts.
Reviewers: martong, espindola, shafik, #lldb, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arsenm, jvesely, nhaehnle, hiraditya, JDevlieghere, teemperor, rnkovacs, emaste, kubamracek, nemanjai, ki.stfu, javed.absar, arichardson, kbarton, jrtc27, MaskRay, atanasyan, dexonsmith, arphaman, jfb, jsji, jdoerfert, lldb-commits, llvm-commits
Tags: #lldb, #llvm
Differential Revision: https://reviews.llvm.org/D61847
llvm-svn: 361484
This reverts commit c28f81797084b8416ff5be4f9e79000a9741ca6a.
This reverts commit 7e79b64642486f510f7872174eb831df68d65b84.
Looks like there is more work to be done on this patch. I've spoken to
the author and for the time being we will revert to keep the buildbots
green.
llvm-svn: 361086
This is a general fix for the ConnectionFileDescriptor class but my main
motivation was to make lldb-server working with IPv6.
The connect URI can use square brackets ([]) to wrap the interface part
of the URI (e.g.: <scheme>://[<interface>]:<port>). For IPv6 addresses
this is a must since its ip can include colons and it will overlap with
the port colon otherwise. The URIParser class parses the square brackets
correctly but the ConnectionFileDescriptor doesn't generate them for
IPv6 addresses making it impossible to connect to the gdb server when
using this protocol.
How to reproduce the issue:
$ lldb-server p --server --listen [::1]:8080
...
$ lldb
(lldb) platform select remote-macosx
(lldb) platform connect connect://[::1]:8080
(lldb) platform process -p <pid>
error: unable to launch a GDB server on 'computer'
The server was actually launched we were just not able to connect to it.
With this fix lldb will correctly connect. I fixed this by wrapping the
ip portion with [].
Differential Revision: https://reviews.llvm.org/D61833
Patch by António Afonso <antonio.afonso@gmail.com>
llvm-svn: 361079
This reverts commit b5a8abd57f23e2f621d5ceb0f64f1bb8f9579c3f. This
should not be needed as the lldb-server tool will add
`LLDB_CAN_USE_LLDB_SERVER` which will never be set to true on Windows.
llvm-svn: 360745
We can piggyback off the existing add_lldb_tool_subdirectory to decide
whether or not lldb-server should be built.
Differential revision: https://reviews.llvm.org/D61872
llvm-svn: 360621
I think the recent change to flush the SB API recording uncovered a real
issue on the Windows bot. Although I couldn't make much sense of the
error message "unknown file: error: SEH exception with code 0x3221225477
thrown in the test body.", it prompted me to look at the test. In the
unit test we were recording during replay, which is obviously not
correct. I think we didn't see this issue before because we flushed once
after the recording was done. This patch unsets the recording logic
during the replay part of the test.
Hopefully this fixed the Windows bot.
llvm-svn: 360298
Previous ArchSpec tests didn't catch this bug since we never tested just the OS being out of date. Fixed the bug and covered this with a test that would catch this.
This was found when trying to load a core file where the core file was an ELF file with just the e_machine for architeture and where the ELF header had no OS set in the OSABI field of the e_ident. It wasn't merging the architecture with the target architecture correctly.
Differential Revision: https://reviews.llvm.org/D61659
llvm-svn: 360292
Remove SymbolVendorMacOSX from the test, as this plugin is not available
on non-mac platforms, and it does not seem to be necessary anyway.
Declare inlined-functions.yaml as an input of the test in cmake.
llvm-svn: 360169
Summary:
This is necessary to support parsing expressions like ".cfa -16 + ^", as
that format is used in breakpad STACK CFI expressions.
Since the PDB expressions use the same parser, this change will affect
them too, but I don't believe that should be a problem in practice. If
PDBs do contain the negative values, it's very likely that they are
intended to be parsed the same way, and if they don't, then it doesn't
matter.
In case that we do ever need to handle this differently, we can always
make the parser behavior customizable, or just use a different parser.
To make sure that the integer size is big enough for everyone, I switch
from using a (unsigned) 32-bit integer to a 64-bit (signed) one.
Reviewers: amccarth, clayborg, aleksandr.urakov
Subscribers: markmentovai, lldb-commits
Differential Revision: https://reviews.llvm.org/D61311
llvm-svn: 360166
Pavel Labath