Summary:
One small step in my long running quest to improve python exception handling in
LLDB. Replace GetInteger() which just returns an int with As<long long> and
friends, which return Expected types that can track python exceptions
Reviewers: labath, jasonmolenda, JDevlieghere, vadimcn
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D78462
Several SB API functions return strings using (char*, size_t) output
arguments. During capture, we serialize an empty string for the char*
because the memory can be uninitialized.
During active replay, we have custom replay redirects that ensure that
we don't override the buffer from which we're reading, but rather write
to a buffer on the heap with the given length. This is sufficient for
the active reproducer use case, where we only care about the side
effects of the API calls, not the values actually returned.
This approach does not not work for passive replay because here we
ignore all the incoming arguments, and re-execute the current function
with the arguments deserialized from the reproducer. This means that
these function will update the deserialized copy of the arguments,
rather than whatever was passed in by the SWIG wrapper.
To solve this problem, this patch extends the reproducer instrumentation
to handle this special case for passive replay. We nog ignore the
replayer in the registry and the incoming char pointer, and instead
reinvoke the current method on the deserialized class, and populate the
output argument.
Differential revision: https://reviews.llvm.org/D77759
This wasn't a great idea to begin with, as you can't really rely on the
implementation, but since it also doesn't work with MSVC I've just made
the ctors public.
Support passive replay as proposed in the RFC [1] on lldb-dev and
described in more detail on the lldb website [2].
This patch extends the LLDB_RECORD macros to re-invoke the current
function with arguments deserialized from the reproducer. This relies on
the function being called in the exact same order as during replay. It
uses the same mechanism to toggle the API boundary as during recording,
which guarantees that only boundary crossing calls are replayed.
Another major change is that before this patch we could ignore the
result of an API call, because we only cared about the observable
behavior. Now we need to be able to return the replayed result to the
SWIG bindings.
We reuse a lot of the recording infrastructure, which can be a little
confusing. We kept the existing naming to limit the amount of churn, but
might revisit that in a future patch.
[1] http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html
[2] https://lldb.llvm.org/resources/reproducers.html
Differential revision: https://reviews.llvm.org/D77602
Summary:
Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.
Bug: llvm.org/PR45310
Depends on D76804.
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: mgorny, jingham, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76805
Fix a bug where UnwindAssemblyInstEmulation would confuse which
register is used to compute the Canonical Frame Address after it
had branched over a mid-function epilogue (where the CFA reg changes
from $fp to $sp in the process of epiloguing). Reinstate the
correct CFA register after we forward the unwind rule for branch
targets. The failure mode was that UnwindAssemblyInstEmulation
would think CFA was set in terms of $sp after one of these epilogues,
and if it sees modifications to $sp after the branch target, it would
change the CFA offset in the unwind rule -- even though the CFA is
defined in terms of $fp and the $sp changes are irrelevant to correct
calculation.
<rdar://problem/60300528>
Differential Revision: https://reviews.llvm.org/D78077
Summary:
Removing the Test prefix from the file name and its usages. The standard is using only Test as a suffix.
This was correctly pointed out in https://reviews.llvm.org/D77444.
Reviewers: labath, clayborg
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77878
The instrumentation unit tests' current implementation uses global
variables to track constructor calls for the instrumented classes during
replay. This is suboptimal because it indirectly relies on how the
reproducer instrumentation is implemented. I found out when adding
support for passive replay and the test broke because we made an extra
(temporary) copy of the instrumented objects.
Additionally, the old approach wasn't very self-explanatory. It took me
a bit of time to understand why we were expecting the number of objects
in the test.
This patch rewrites the test and uses the index-to-object-mapping to
verify the objects created during replay. You can now specify the
expected objects, in order, and whether they should be valid or not. I
find that it makes the tests much easier to understand. More
importantly, this approach is resilient to implementation detail changes
in the instrumentation.
Types that came from a Clang module are nested in DW_TAG_module tags
in DWARF. This patch recreates the Clang module hierarchy in LLDB and
1;95;0csets the owning module information accordingly. My primary motivation
is to facilitate looking up per-module APINotes for individual
declarations, but this likely also has other applications.
This reapplies the previously reverted commit, but without support for
ClassTemplateSpecializations, which I'm going to look into separately.
rdar://problem/59634380
Differential Revision: https://reviews.llvm.org/D75488
The synchronization logic in the previous had a subtle bug. Moving of
the "m_read_thread_did_exit = true" into the critical section made it
possible for some threads calling SynchronizeWithReadThread call to get
stuck. This could happen if there were already past the point where they
checked this variable. In that case, they would block on waiting for the
eBroadcastBitNoMorePendingInput event, which would never come as the
read thread was blocked on getting the synchronization mutex.
The new version moves that line out of the critical section and before
the sending of the eBroadcastBitNoMorePendingInput event, and also adds
some comments to explain why the things need to be in this sequence:
- m_read_thread_did_exit = true: prevents new threads for waiting on
events
- eBroadcastBitNoMorePendingInput: unblock any current thread waiting
for the event
- Disconnect(): close the connection. This is the only bit that needs to
be in the critical section, and this is to ensure that we don't close
the connection while the synchronizing thread is mucking with it.
Original commit message follows:
Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.
However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.
This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.
The fix is to hold the synchronization mutex while closing the
connection.
I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.
The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.
Reviewers: clayborg, JDevlieghere
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77295
Summary:
Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.
However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.
This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.
The fix is to hold the synchronization mutex while closing the
connection.
I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.
The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.
Reviewers: clayborg, JDevlieghere
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77295
Summary:
This adds support for commands created through the API to support autorepeat.
This covers the case of single word and multiword commands.
Comprehensive tests are included as well.
Reviewers: labath, clayborg
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77444
This is mostly useful for Swift support; it allows LLDB to substitute
a matching SDK it shipped with instead of the sysroot path that was
used at compile time.
The goal of this is to make the Xcode SDK something that behaves more
like the compiler's resource directory, as in that it ships with LLDB
rather than with the debugged program. This important primarily for
importing Swift and Clang modules in the expression evaluator, and
getting at the APINotes from the SDK in Swift.
For a cross-debugging scenario, this means you have to have an SDK for
your target installed alongside LLDB. In Xcode this will always be the
case.
rdar://problem/60640017
Differential Revision: https://reviews.llvm.org/D76471
LLDB relies on empty FileSpecs being invalid files, for example, they
don't exists. Currently this assumption does not always hold during
reproducer replay, because we pass the result of GetPath to the VFS.
This is an empty string, which the VFS converts to an absolute directory
by prepending the current working directory, before looking it up in the
YAML mapping. This means that an empty FileSpec will exist when the
current working directory does. This breaks at least one test
(TestAddDsymCommand.py) when ran from replay.
This patch special cases empty FileSpecs and returns a sensible result
before calling GetPath and forwarding the call.
Differential revision: https://reviews.llvm.org/D77351
Types that came from a Clang module are nested in DW_TAG_module tags
in DWARF. This patch recreates the Clang module hierarchy in LLDB and
sets the owning module information accordingly. My primary motivation
is to facilitate looking up per-module APINotes for individual
declarations, but this likely also has other applications.
rdar://problem/59634380
Differential Revision: https://reviews.llvm.org/D75488
This patch fixes a crash that happens on the DWARF expression evaluator
when trying to access the top of the stack while it's empty.
rdar://60512489
Differential Revision: https://reviews.llvm.org/D77108
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch fixes a crash that happens on the DWARF expression evaluator
when trying to access the top of the stack while it's empty.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
CPlusPlusNameParser is used in several places on of them is during IR execution and setting breakpoints to pull information C++ like the basename, the context and arguments.
Currently it does not handle templated operator< properly, because of idiosyncrasy is how clang generates debug info for these cases.
It uses clang::Lexer which will tokenize operator<<A::B> into:
tok::kw_operator
tok::lessless
tok::raw_identifier
Later on the parser in ConsumeOperator() does not handle this case properly and we end up failing to parse.
Differential Revision: https://reviews.llvm.org/D76168
Summary:
When using IPv6 host:port pairs, typically the host is put inside
brackets, such as [2601🔢...:0213]:5555, and the UriParser
can handle this format.
However, the Android infrastructure in LLDB assumes an additional
brackets around the host:port pair, such that the entire host:port
string can be treated as the host (which is used as an Android Serial
Number), and UriParser cannot handle multiple brackets. Parsing
inputs with such extra backets requires searching the closing bracket
from the right.
Test: BracketedHostnameWithPortIPv6 covers the case mentioned above
Reviewers: #lldb, labath
Reviewed By: labath
Subscribers: kwk, shafik, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76736
This patch changes the way the StackFrame Recognizers match a certain
frame.
Until now, recognizers could be registered with a function
name but also an alternate symbol.
This change is motivated by a test failure for the Assert frame
recognizer on Linux. Depending the version of the libc, the abort
function (triggered by an assertion), could have more than two
signatures (i.e. `raise`, `__GI_raise` and `gsignal`).
Instead of only checking the default symbol name and the alternate one,
lldb will iterate over a list of symbols to match against.
rdar://60386577
Differential Revision: https://reviews.llvm.org/D76188
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
The current implementation isn't very resilient when it comes to the
output of xcrun. Currently it cannot deal with:
- Trailing newlines.
- Leading newlines and errors/warnings before the Xcode path.
- Xcode not being named Xcode.app.
This extract the logic into a helper in PlatformDarwin and fixes those
issues. It's also the first step towards removing code duplication
between the different platforms and downstream Swift.
Differential revision: https://reviews.llvm.org/D76261
Add YAML traits for ArchSpec and ProcessInstanceInfo so they can be
serialized for the reproducers.
Differential revision: https://reviews.llvm.org/D76004
Add YAML traits for the ConstString and FileSpec classes so they can be
serialized as part of ProcessInfo. The latter needs to be serializable
for the reproducers.
Differential revision: https://reviews.llvm.org/D76002
offset_t is unsigned, so if the RHS is signed we get a warning from clang:
warning: comparison of integers of different signs: 'const unsigned long long' and 'const int'
If a producer emits a nonzero segment size, `lldb` will silently read
incorrect values and crash, or do something worse later as the tuple
size is expected to be 2, rather than 3.
Neither LLVM, nor GCC produce segmented aranges, but this dangerous case
should still be checked and handled.
Reviewed by: clayborg, labath
Differential Revision: https://reviews.llvm.org/D75925
Subscribers: lldb-commits
Tags: #lldb
Summary: This adds unit tests for FindEntryIndexesThatContain, this is done in preparation for changing the logic of the function.
Reviewers: labath, teemperor
Reviewed By: labath
Subscribers: arphaman, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75180
Summary:
This packet is necessary to make lldb work with the remote-gdb stub in
user mode qemu when running position-independent binaries. It reports
the relative position (load bias) of the loaded executable wrt. the
addresses in the file itself.
Lldb needs to know this information in order to correctly set the load
address of the executable. Normally, lldb would be able to find this out
on its own by following the breadcrumbs in the process auxiliary vector,
but we can't do this here because qemu does not support the
qXfer:auxv:read packet.
This patch does not implement full scope of the qOffsets packet (it only
supports packets with identical code, data and bss offsets), because it
is not fully clear how should the different offsets be handled and I am
not aware of a producer which would make use of this feature (qemu will
always
<https://github.com/qemu/qemu/blob/master/linux-user/elfload.c#L2436>
return the same value for code and data offsets). In fact, even gdb
ignores the offset for the bss sections, and uses the "data" offset
instead. So, until the we need more of this packet, I think it's best
to stick to the simplest solution possible. This patch simply rejects
replies with non-uniform offsets.
Reviewers: clayborg, jasonmolenda
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74598
AVR usually uses two byte addresses. By making DataExtractor deal with
this, it is possible to load AVR binaries that don't have debug info
associated with them.
Differential Revision: https://reviews.llvm.org/D73969
Since f9568a9549 this function takes a
CompilerDeclContext reference instead of a pointer. It overlooked this function
when I fixed the compilation for FindTypes.
LLDB has a few different styles of header guards and they're not very
consistent because things get moved around or copy/pasted. This patch
unifies the header guards across LLDB and converts everything to match
LLVM's style.
Differential revision: https://reviews.llvm.org/D74743
This patch changes the way we initialize and terminate the plugins in
the system initializer. It uses an approach similar to LLVM's
TARGETS_TO_BUILD with a def file that enumerates the plugins.
The previously landed patch got reverted because it was lacking:
(1) A plugin definition for the Objective-C language runtime,
(2) The dependency between the Static and WASM dynamic loader,
(3) Explicit initialization of ScriptInterpreterNone for lldb-test.
All issues have been addressed in this patch.
Differential revision: https://reviews.llvm.org/D73067
This patch changes the way we initialize and terminate the plugins in
the system initializer. It uses an approach similar to LLVM's
TARGETS_TO_BUILD with a def file that enumerates the plugins.
Differential revision: https://reviews.llvm.org/D73067
Summary:
This patch removes the bitrotted SymbolFileDWARF(Dwo)Dwp classes, and
replaces them with dwp support implemented directly inside
SymbolFileDWARFDwo, in a manner mirroring the implementation in llvm.
This patch does:
- add support for the .debug_cu_index section to our DWARFContext
- adds a llvm::DWARFUnitIndex argument to the DWARFUnit constructors.
This argument is used to look up the offsets of the debug_info and
debug_abbrev contributions in the sections of the dwp file.
- makes sure the creation of the DebugInfo object as well as the initial
discovery of DWARFUnits is thread-safe, as we can now call this
concurrently when doing parallel indexing.
This patch does not:
- use the DWARFUnitIndex to search for other kinds of contributions
(debug_loc, debug_ranges, etc.). This means that units which reference
these sections will not work correctly. These will be handled by
follow-up patches, but even the present level of support is sufficient
to enable basic functionality.
- Make the llvm::DWARFContext thread-safe. Right now, it just avoids this
problem by ensuring everything is initialized ahead of time. However,
this is something we will run into more often as we try to use more of
llvm, and so I plan to start looking into our options here.
Reviewers: JDevlieghere, aprantl, clayborg
Subscribers: mgorny, mgrang, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73783
Summary:
These definitions are used to "augment" information received from the remote
target with eh/debug frame and "generic" register numbers.
Besides being verbose, this information was also incomplete (new registers like
xmm16-31 were missing) and sometimes even downright wrong (ymm register
numbers).
Most of this information is available via llvm's MCRegisterInfo. This patch
creates a new class, MCBasedABI, which retrieves the eh and debug frame register
numbers this way. The tricky part here is that the llvm class uses all-caps
register names, whereas lldb register are lowercase, and sometimes called
slightly differently. Therefore this class introduces some hooks to allow a
subclass to customize the MC lookup. The subclass also needs to suply the
"generic" register numbers, as this is an lldb invention.
This patch ports the x86_64 ABI classes to use the new register info mechanism.
It also creates a new "ABIx86_64" class which can be used to house code common
to x86_64 both ABIs. Right now, this just consists of a single function, but
there are plenty of other things that could be moved here too.
Reviewers: JDevlieghere, jasonmolenda
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74244
Summary:
The only use of this class was to implement the SharedCluster of ValueObjects.
However, the same functionality can be implemented using a regular
std::shared_ptr, and its little-known "sub-object pointer" feature, where the
pointer can point to one thing, but actually delete something else when it goes
out of scope.
This patch reimplements SharedCluster using this feature --
SharedClusterPointer::GetObject now returns a std::shared_pointer which points
to the ValueObject, but actually owns the whole cluster. The only change I
needed to make here is that now the SharedCluster object needs to be created
before the root ValueObject. This means that all private ValueObject
constructors get a ClusterManager argument, and their static Create functions do
the create-a-manager-and-pass-it-to-value-object dance.
Reviewers: teemperor, JDevlieghere, jingham
Subscribers: mgorny, jfb, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74153
This reimplements commit 6b2979c123 and updates
the tests to reflect the addition of the alternate symbol attribute.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
D73303 was failing on Fedora Linux and so it was disabled by Skip the
AssertFrameRecognizer test for Linux.
I find no easy way how to find out if it gets recognized as
`__assert_fail` or `__GI___assert_fail` as during `Process` ctor
libc.so.6 is not yet loaded by the debuggee.
DWARF symbol `__GI___assert_fail` overrides the ELF symbol `__assert_fail`.
While external debug info (=DWARF) gets disabled for testsuite (D55859)
that sure does not apply for real world usage.
Differential Revision: https://reviews.llvm.org/D74252
One way to register a recognizer is to use RegularExpressionSP for the
module and symbol.
In order to match a symbol regardless of the module, the recognizer can
be registered with a nullptr for the module. However, this cause the
frame recognizer list command to crash because it calls
RegularExpression::GetText without checking if the shared pointer is valid.
This patch adds checks for the symbol and module RegularExpressionSP.
Differential Revision: https://reviews.llvm.org/D74212
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
lldb-forward.h is convenient in many ways, but having clang-based
class forward declarations in there makes it easy to proliferate uses of clang
outside of plugins. Removing them makes you much more conscious of when
you're using something from clang and marks where we're using things
from clang in non-plugins.
Differential Revision: https://reviews.llvm.org/D73935
DataExtractor::GetMaxS64Bitfield performs a shift with UB in order to
construct a bitmask when bitfield_bit_size is 64. The current
implementation actually does “work” in this case, because the assumption
that the shift result is 0 holds, and 0 minus 1 gives the all-ones value
(the correct mask). However, the more readable/maintainable approach
might be to use an off-the-shelf UB-free helper.
Fixes a UBSan issue:
"col" : 37,
"description" : "invalid-shift-exponent",
"filename" : "/Users/vsk/src/llvm-project-master/lldb/source/Utility/DataExtractor.cpp",
"instrumentation_class" : "UndefinedBehaviorSanitizer",
"line" : 615,
"memory_address" : 0,
"summary" : "Shift exponent 64 is too large for 64-bit type 'uint64_t' (aka 'unsigned long long')",
rdar://59117758
Differential Revision: https://reviews.llvm.org/D73913
Summary:
This change represents the move of ClangASTImporter, ClangASTMetadata,
ClangExternalASTSourceCallbacks, ClangUtil, CxxModuleHandler, and
TypeSystemClang from lldbSource to lldbPluginExpressionParserClang.h
This explicitly removes knowledge of clang internals from lldbSymbol,
moving towards a more generic core implementation of lldb.
Reviewers: JDevlieghere, davide, aprantl, teemperor, clayborg, labath, jingham, shafik
Subscribers: emaste, mgorny, arphaman, jfb, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73661
Use the std::string conversion operator introduced in
d7049213d0. The SmallString in the log
statement doesn't require conversion at all when using the variadic log
macro.
Summary:
I noticed this strange line in `ASTImporterDelegate::ImportDefinitionTo` which doesn't make a lot of sense:
```
to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
```
It forcibly sets the imported TagDecl to be defined if the source TagDecl was defined. This doesn't make any
sense as in this code we already forced the ASTImporter to import the definition so this should always be
a no-op.
Turns out this is hiding two bugs:
1. The way we handle forward declarations in the debug info that might be completed later is that we
import them and then mark them as having external lexical storage. This makes Clang ask for the definition
later when it needs it (at which point we hopefully have the definition around and can complete it). However,
this is currently not completing the forward decls with external storage but instead creates a duplicated decl
in the target AST which is then defined. The forward decl is kept incomplete after the import and we just
forcibly make it a definition of the record without any content with our workaround. The TestSharedLib* tests
is only passing because of this.
2. Minimal import of lambdas is broken and never imports the definition it seems. That appears to be a bug
in the ASTImporter which gives the definition of lambda's some special treatment. TestLambdas.py is actually
broken but is passing because of this workaround.
This patch fixes the first bug by forcing the ASTImporter to import to the target forward declaration. We can't
delete the workaround as the second bug is still around but that will be a follow up review for the ASTImporter.
However it will get rid of all the duplicated RecordDecls that are in our expression AST that are strangely defined
but don't have any of the fields they are supposed to have.
Reviewers: shafik, labath
Reviewed By: shafik
Subscribers: aprantl, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73345
This has the same behavior as converting std::string_view to
std::string. This is an expensive conversion, so explicit conversions
are helpful for avoiding unneccessary string copies.
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
The GetRawLine currently returns the full command line used
to create the CompletionRequest. So for example for "foo b[tab] --arg"
it would return the whole string instead of "foo b". Usually
completion code makes the wrong assumption that the cursor is at
the end of the line and handing out the complete line will cause
that people implement completions that also make this assumption.
This patch makes GetRawLine() return only the string until the
cursor and hides the suffix (so that the cursor is always at the
end of this string) and adds another function GetRawLineWithUnusedSuffix
that is specifically the line with the suffix that isn't used by
the CompletionRequest for argument parsing etc.
There is only one user of this new function that actually needs the
suffix and that is the expression command which needs the suffix to
detect if it is in the raw or argument part of the command (by looking
at the "--" separator).
Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).
This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).
Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73258
Summary:
This commit renames ClangASTContext to TypeSystemClang to better reflect what this class is actually supposed to do
(implement the TypeSystem interface for Clang). It also gets rid of the very confusing situation that we have both a
`clang::ASTContext` and a `ClangASTContext` in clang (which sometimes causes Clang people to think I'm fiddling
with Clang's ASTContext when I'm actually just doing LLDB work).
I also have plans to potentially have multiple clang::ASTContext instances associated with one ClangASTContext so
the ASTContext naming will then become even more confusing to people.
Reviewers: #lldb, aprantl, shafik, clayborg, labath, JDevlieghere, davide, espindola, jdoerfert, xiaobai
Reviewed By: clayborg, labath, xiaobai
Subscribers: wuzish, emaste, nemanjai, mgorny, kbarton, MaskRay, arphaman, jfb, usaxena95, jingham, xiaobai, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72684
Summary:
I often struggle to understand what exactly LLDB is doing by looking at our expression evaluation logging as our messages look like this:
```
CompleteTagDecl[2] on (ASTContext*)0x7ff31f01d240 Completing (TagDecl*)0x7ff31f01d568 named DeclName1
```
From the log messages it's unclear what this ASTContext is. Is it the scratch context, the expression context, some decl vendor context or a context from a module?
The pointer value isn't helpful for anyone unless I'm in a debugger where I could inspect the memory at the address. But even with a debugger it's not easy to
figure out what this ASTContext is without having deeper understanding about all the different ASTContext instances in LLDB (e.g., valid SourceLocation
from the file system usually means that this is the Objective-C decl vendor, a file name from multiple expressions is probably the scratch context, etc.).
This patch adds a name field to ClangASTContext instances that we can use to store a name which can be used for logging and debugging. With this
our log messages now look like this:
```
CompleteTagDecl[2] on scratch ASTContext. Completing (TagDecl*)0x7ff31f01d568 named Foo
```
We can now also just print a ClangASTContext from the debugger and see a useful name in the `m_display_name` field, e.g.
```
m_display_name = "AST for /Users/user/test/main.o";
```
Reviewers: shafik, labath, JDevlieghere, mib
Reviewed By: shafik
Subscribers: clayborg, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72391
Summary:
CXXRecordDecls that have a move constructor but no copy constructor need to
have their implicit copy constructor marked as deleted (see C++11 [class.copy]p7, p18)
Currently we don't do that when building an AST with ClangASTContext which causes
Sema to realise that the AST is malformed and asserting when trying to create an implicit
copy constructor for us in the expression:
```
Assertion failed: ((data().DefaultedCopyConstructorIsDeleted || needsOverloadResolutionForCopyConstructor())
&& "Copy constructor should not be deleted"), function setImplicitCopyConstructorIsDeleted, file include/clang/AST/DeclCXX.h, line 828.
```
In the test case there is a class `NoCopyCstr` that should have its copy constructor marked as
deleted (as it has a move constructor). When we end up trying to tab complete in the
`IndirectlyDeletedCopyCstr` constructor, Sema realises that the `IndirectlyDeletedCopyCstr`
has no implicit copy constructor and tries to create one for us. It then realises that
`NoCopyCstr` also has no copy constructor it could find via lookup. However because we
haven't marked the FieldDecl as having a deleted copy constructor the
`needsOverloadResolutionForCopyConstructor()` returns false and the assert fails.
`needsOverloadResolutionForCopyConstructor()` would return true if during the time we
added the `NoCopyCstr` FieldDecl to `IndirectlyDeletedCopyCstr` we would have actually marked
it as having a deleted copy constructor (which would then mark the copy constructor of
`IndirectlyDeletedCopyCstr ` as needing overload resolution and Sema is happy).
This patch sets the correct mark when we complete our CXXRecordDecls (which is the time when
we know whether a copy constructor has been declared). In theory we don't have to do this if
we had a Sema around when building our debug info AST but at the moment we don't have this
so this has to do the job for now.
Reviewers: shafik
Reviewed By: shafik
Subscribers: aprantl, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72694
and document the shortcomings of LLDB's partially defined DW_OP_piece
handling.
This would manifest as "DW_OP_piece for offset foo but top of stack is
of size bar".
rdar://problem/46262998
Differential Revision: https://reviews.llvm.org/D72880
By switching to Scalars that are backed by explicitly-sized APInts we
can avoid a bug that increases the buffer reserved for a small piece
to the next-largest host integer type.
This manifests as "DW_OP_piece for offset foo but top of stack is of size bar".
Differential Revision: https://reviews.llvm.org/D72879
Summary:
In Debug builds we call VerifyDecl in ClangASTContext::CreateFunctionDeclaration which in turn
calls `getAccess` on the created FunctionDecl. As we passed in a RecordDecl as the DeclContext
for the FunctionDecl, we end up hitting the assert in `getAccess` that checks that we never have
a Decl inside a Record without a valid AccessSpecifier. FunctionDecls are never in RecordDecls
(that would be a CXXMethodDecl) so setting a access specifier would not be the correct way to
fix this.
Instead this patch does the same thing that DWARFASTParserClang::ParseSubroutine is doing:
We pass in the FunctionDecl with the TranslationUnit as the DeclContext. That's not ideal but
it is how we currently do it when creating our debug info AST, so the unit test should do
the same.
Reviewers: shafik
Reviewed By: shafik
Subscribers: aprantl, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72359
As suggested by @labath extended RangeDataVector so that user can provide
custom sorting of the Entry's `data' field for D63540.
https://reviews.llvm.org/D63540
RangeData functions were used just by RangeDataVector (=after I removed them
LLDB still builds fine) which no longer uses them so I removed them.
Differential revision: https://reviews.llvm.org/D72460
Creating an ASTContext with an unknown triple is rarely a good idea (as usually
all our ASTs have a valid triple that is either from the host or the target) and the
default argument makes it far to easy to implicitly create such an AST. Let's
remove it and force people to pass a triple.
The only place where we don't pass a triple is a DWARFASTParserClangTests
where we now just pass the host triple instead (the test doesn't depend on any
triple so this shouldn't change anything).
This constructor is supposed to take a string representing an llvm::Triple.
We might as well take a llvm::Triple here which saves us all the string
conversions in the call sites and we make this more type safe.
Summary:
We currently don't set access specifiers for function template declarations. This seems to be fine
as long as the function template is not declared inside any record in which case Clang asserts
with the following once we try to query it's access:
```
Assertion failed: (Access != AS_none && "Access specifier is AS_none inside a record decl"), function AccessDeclContextSanity,
```
This patch just marks these function template declarations as public to make Clang happy.
Reviewers: shafik, teemperor
Reviewed By: teemperor
Subscribers: JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71909
This adds a check that the ClangASTContext actually fits to the
DeclContext that we want to create a CompilerDeclContext for. If
the ClangASTContext (and its associated ASTContext) does not fit
to the DeclContext (that is, the DeclContext wasn't created by the
ASTContext), all computations using this malformed CompilerDeclContext
will yield unpredictable results.
Also fixes the only place that actually hits this assert which is the
construction of a CompilerDeclContext in ClangExpressionDeclMap
where we pass an unrelated ASTContext instead of the ASTContext
of the current expression.
I had to revert my previous change to DWARFASTParserClangTests.cpp
back to using the unsafe direct construction of CompilerDeclContext
as this assert won't work if the DeclContext we pass isn't a valid
DeclContext in the first place.
Summary:
Many of our tests need to initialize certain subsystems/plugins of LLDB such as
`FileSystem` or `HostInfo` by calling their static `Initialize` functions before the
test starts and then calling `::Terminate` after the test is done (in reverse order).
This adds a lot of error-prone boilerplate code to our testing code.
This patch adds a RAII called SubsystemRAII that ensures that we always call
::Initialize and then call ::Terminate after the test is done (and that the Terminate
calls are always in the reverse order of the ::Initialize calls). It also gets rid of
all of the boilerplate that we had for these calls.
Per-fixture initialization is still not very nice with this approach as it would
require some kind of static unique_ptr that gets manually assigned/reseted
from the gtest SetUpTestCase/TearDownTestCase functions. Because of that
I changed all per-fixture setup to now do per-test setup which can be done
by just having the SubsystemRAII as a member of the test fixture. This change doesn't
influence our normal test runtime as LIT anyway runs each test case separately
(and the Initialize/Terminate calls are anyway not very expensive). It will however
make running all tests in a single executable slightly slower.
Reviewers: labath, JDevlieghere, martong, espindola, shafik
Reviewed By: labath
Subscribers: mgorny, rnkovacs, emaste, MaskRay, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71630
The CompilerDeclContext constructor takes a void* pointer which
means that all callers of this constructor need to first explicitly
convert all pointers to clang::DeclContext*. This causes that we
for example can't just pass a TranslationUnitDecl* to the constructor without
first casting it to its parent class (as it inherits from both
Decl and DeclContext so the void* pointer is actually a Decl*).
This patch introduces a utility function in the ClangASTContext
which gets rid of the requirement to cast all pointers to
clang::DeclContext. Also moves all constructor calls to use this
function instead which is NFC (beside the change in
DWARFASTParserClangTests.cpp).
ClangASTContext::getASTContext() currently returns a ptr but we have an assert there since a
while that the ASTContext is not a nullptr. This causes that we still have a lot of code
that is doing nullptr checks on the result of getASTContext() which is all unreachable code.
This patch changes the return value to a reference to make it clear this can't be a nullptr
and deletes all the nullptr checks.
Their naming is misleading as they only return the
ClangASTContext-owned variables. For ClangASTContext instances constructed
for a given clang::ASTContext they silently generated duplicated instances
(e.g., a second IdentifierTable) that were essentially unusable.
This removes all these getters as they are anyway not very useful in comparison
to just calling the clang::ASTContext getters. The initialization
code has been moved to the CreateASTContext initialization method so that all
code for making our own clang::ASTContext is in one place.
This implements a very elementary Lua script interpreter. It supports
running a single command as well as running interactively. It uses
editline if available. It's still missing a bunch of stuff though. Some
things that I intentionally ingored for now are that I/O isn't properly
hooked up (so every print goes to stdout) and the non-editline support
which is not handling a bunch of corner cases. The latter is a matter of
reusing existing code in the Python interpreter.
Discussion on the mailing list:
http://lists.llvm.org/pipermail/lldb-dev/2019-December/015812.html
Differential revision: https://reviews.llvm.org/D71234
Pavel Labath