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
Checking this in for Antonio Afonso:
This diff changes the function LineEntry::GetSameLineContiguousAddressRange so that it also includes function calls that were inlined at the same line of code.
My motivation is to decrease the step over time of lines that heavly rely on inlined functions. I have multiple examples in the code base I work that makes a step over stop 20 or mote times internally. This can easly had up to step overs that take >500ms which I was able to lower to 25ms with this new strategy.
The reason the current code is not extending the address range beyond an inlined function is because when we resolve the symbol at the next address of the line entry we will get the entry line corresponding to where the original code for the inline function lives, making us barely extend the range. This then will end up on a step over having to stop multiple times everytime there's an inlined function.
To check if the range is an inlined function at that line I also get the block associated with the next address and check if there is a parent block with a call site at the line we're trying to extend.
To check this I created a new function in Block called GetContainingInlinedBlockWithCallSite that does exactly that. I also added a new function to Declaration for convinence of checking file/line named CompareFileAndLine.
To avoid potential issues when extending an address range I added an Extend function that extends the range by the AddressRange given as an argument. This function returns true to indicate sucess when the rage was agumented, false otherwise (e.g.: the ranges are not connected). The reason I do is to make sure that we're not just blindly extending complete_line_range by whatever GetByteSize() we got. If for some reason the ranges are not connected or overlap, or even 0, this could be an issue.
I also added a unit tests for this change and include the instructions on the test itself on how to generate the yaml file I use for testing.
Differential Revision: https://reviews.llvm.org/D61292
llvm-svn: 360071
Summary:
According to [C128] "Virtual functions should specify exactly one
of `virtual`, `override`, or `final`", I've added override where a
virtual function is overriden but the explicit `override` keyword
was missing. Whenever both `virtual` and `override` were specified,
I removed `virtual`. As C.128 puts it:
> [...] writing more than one of these three is both redundant and
> a potential source of errors.
I anticipate a discussion about whether or not to add `override` to
destructors but I went for it because of an example in [ISOCPP1000].
Let me repeat the comment for you here:
Consider this code:
```
struct Base {
virtual ~Base(){}
};
struct SubClass : Base {
~SubClass() {
std::cout << "It works!\n";
}
};
int main() {
std::unique_ptr<Base> ptr = std::make_unique<SubClass>();
}
```
If for some odd reason somebody removes the `virtual` keyword from the
`Base` struct, the code will no longer print `It works!`. So adding
`override` to destructors actively protects us from accidentally
breaking our code at runtime.
[C128]: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final
[ISOCPP1000]: https://github.com/isocpp/CppCoreGuidelines/issues/1000#issuecomment-476951555
Reviewers: teemperor, JDevlieghere, davide, shafik
Reviewed By: teemperor
Subscribers: kwk, arphaman, kadircet, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61440
llvm-svn: 359868
Summary:
This node represents can be used to refer to the initial value, which is
sometimes pushed onto the DWARF stack as the "input" to the DWARF
expression. The typical use case (and the reason why I'm introducing it)
is that the "Canonical Frame Address" is passed this way to the DWARF
expressions computing the values of registers during frame unwind.
The nodes are converted into dwarf by keeping track of DWARF stack depth
an any given point, and then copying the initial value from the bottom
of the stack via the DW_OP_pick opcode. This could be made more
efficient for simple expressions, but here I chose to start with the
most general implementation possible.
Reviewers: amccarth, clayborg, aleksandr.urakov
Subscribers: aprantl, jasonmolenda, lldb-commits, markmentovai
Differential Revision: https://reviews.llvm.org/D61183
llvm-svn: 359560
Their functionality overlaps with the newly introduced
PostfixExpressionTests (r359288). Tests, which still exercise some
pdb-related functionality (register name resolution) have been kept.
llvm-svn: 359450
Summary:
The DWARF spec states that the DWARF stack arguments are numbered from
the top. Our implementation of DW_OP_pick was counting them from the
bottom.
This bug probably wasn't noticed because nobody (except my upcoming
postfix-to-DWARF converter) uses DW_OP_pick, but I've cross-checked with
gdb to confirm that counting from the top is the expected behavior.
This patch fixes the implementation to match the spec and gdb behavior
and adds a test.
Reviewers: jasonmolenda, clayborg
Subscribers: mgorny, aprantl, lldb-commits
Differential Revision: https://reviews.llvm.org/D61182
llvm-svn: 359436
Summary:
The new dwarf generator is pretty much a verbatim copy of the one in
PDB.
In order to write a pdb-independent test for it, I needed to write a
dummy "symbol resolver", which (together with the fact that I'll need
one more for breakpad-specific resolution logic) prompted me to create a
more simple interface for algorithms which replace or "resolve"
SymbolNodes. The resolving algorithms in NativePDB have been updated to
make use of that too.
I have removed a couple of NativePDB tests which weren't testing
anything pdb-specific and where the tested functionality was covered by
the new format-agnostic tests I have added.
Reviewers: amccarth, clayborg, aleksandr.urakov
Subscribers: aprantl, markmentovai, lldb-commits, jasonmolenda, JDevlieghere
Differential Revision: https://reviews.llvm.org/D61056
llvm-svn: 359288
Summary:
When we want to compare a ConstString against a string literal (or any other non-ConstString),
we currently have to explicitly turn the other string into a ConstString. This makes sense as
comparing ConstStrings against each other is only a fast pointer comparison.
However, currently we (rather incorrectly) use in several places in LLDB temporary ConstStrings when
we just want to compare a given ConstString against a hardcoded value, for example like this:
```
if (extension != ConstString(".oat") && extension != ConstString(".odex"))
```
Obviously this kind of defeats the point of ConstStrings. In the comparison above we would
construct two temporary ConstStrings every time we hit the given code. Constructing a
ConstString is relatively expensive: we need to go to the StringPool, take a read and possibly
an exclusive write-lock and then look up our temporary string in the string map of the pool.
So we do a lot of heavy work for essentially just comparing a <6 characters in two strings.
I initially wanted to just fix these issues by turning the temporary ConstString in static variables/
members, but that made the code much less readable. Instead I propose to add a new overload
for the ConstString comparison operator that takes a StringRef. This comparison operator directly
compares the ConstString content against the given StringRef without turning the StringRef into
a ConstString.
This means that the example above can look like this now:
```
if (extension != ".oat" && extension != ".odex")
```
It also no longer has to unlock/lock two locks and call multiple functions in other TUs for constructing
the temporary ConstString instances. Instead this should end up just being a direct string comparison
of the two given strings on most compilers.
This patch also directly updates all uses of temporary and short ConstStrings in LLDB to use this new
comparison operator. It also adds a some unit tests for the new and old comparison operator.
Reviewers: #lldb, JDevlieghere, espindola, amccarth
Reviewed By: JDevlieghere, amccarth
Subscribers: amccarth, clayborg, JDevlieghere, emaste, arichardson, MaskRay, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D60667
llvm-svn: 359281
Summary:
The postfix expressions in PDB and breakpad symbol files are similar
enough that they can be parsed by the same parser. This patch
generalizes the parser in the NativePDB plugin and moves it into the
PostfixExpression file created in the previous commit (r358976).
The generalization consists of treating any unrecognised token as a
"symbol" node (previously these would only be created for tokens
starting with "$", and other token would abort the parse). This is
needed because breakpad symbols can also contain ".cfa" tokens, which
refer to the frame's CFA.
The cosmetic changes include:
- using a factory function instead of a class for creating nodes (this
is more generic as it allows the same BumpPtrAllocator to be used for
other things too)
- using dedicated function for parsing operator tokens instead of a
DenseMap (more efficient as we don't need to create the DenseMap every
time).
Reviewers: amccarth, clayborg, JDevlieghere, aleksandr.urakov
Subscribers: jasonmolenda, lldb-commits, markmentovai, mgorny
Differential Revision: https://reviews.llvm.org/D61003
llvm-svn: 359073
The tests reading the untouched module list are now not using any lldb
code (as module list loading lives in llvm now), so they can be removed.
The "filtering" of the module list remains (and probably will remain) an
lldb concept, so I keep those tests, but replace the checked-in binaries
with their yaml equivalents.
The binaries which are no longer referenced by any tests have been
removed.
llvm-svn: 358850
LLVM's wchar to UTF8 conversion routine expects an empty string to store the output.
GetHostName() on Windows is sometimes called with a non-empty string which triggers
an assert. The simple fix is to clear the output string before the conversion.
llvm-svn: 358550
D59433 and D60501 changed the way UUIDs are computed from minidump
files. This was done to synchronize the U(G)UID representation with the
native tools of given platforms, but it created a mismatch between
minidumps and breakpad files.
This updates the breakpad algorithm to match the one found in minidumps,
and also adds a couple of tests which should fail if these two ever get
out of sync. Incidentally, this means that the module id in the breakpad
files is almost identical to our notion of UUIDs, so the computation
algorithm can be somewhat simplified.
llvm-svn: 358500
A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
In this patch, I just remove the structure definitions for the
ModuleList stream and the associated parsing code. The rest of the code
is converted to work with the definitions in llvm. NFC.
llvm-svn: 358070
Summary:
This patch adds support for parsing STACK CFI records from breakpad
files. The expressions specifying the values of registers are not
parsed.The idea is that these will be handed off to the postfix
expression -> dwarf compiler, once it is extracted from the internals of
the NativePDB plugin.
Reviewers: clayborg, amccarth, markmentovai
Subscribers: aprantl, lldb-commits
Differential Revision: https://reviews.llvm.org/D60268
llvm-svn: 357975
I also update the tests for SystemInfo parsing to use the yaml2minidump
capabilities in llvm instead of relying on checked-in binaries.
llvm-svn: 357896
This patch removes the lower layers of the minidump parsing code from
the MinidumpParser class, and replaces it with the minidump parser in
llvm.
Not all functionality is already avaiable in the llvm class, but it is
enough for us to be able to stop enumerating streams manually, and rely
on the minidump directory parsing code from the llvm class.
This also removes some checked-in binaries which were used to test error
handling in the parser, as the error handling is now done (and tested)
in llvm. Instead I just add one test that ensures we correctly propagate
the errors reported by the llvm parser. The input for this test can be
written in yaml instead of a checked-in binary.
llvm-svn: 357748
Previously we would classify all STACK records into a single bucket.
This is not really helpful, because there are three distinct types of
records beginning with the token "STACK" (STACK CFI INIT, STACK CFI,
STACK WIN). To be consistent with how we're treating other records, we
should classify these as three different record types.
It also implements the logic to put "STACK CFI INIT" and "STACK CFI"
records into the same "section" of the breakpad file, as they are meant
to be read together (similar to how FUNC and LINE records are treated).
The code which performs actual parsing of these records will come in a
separate patch.
llvm-svn: 357691
Adrian Prantl