After this patch all remaining tests should pass on macOS when replayed
from a reproducer.
To capture the reproducers:
./bin/llvm-lit ../llvm-project/lldb/test/ --param lldb-run-with-repro=capture
To replay the reproducers:
./bin/llvm-lit ../llvm-project/lldb/test/ --param lldb-run-with-repro=replay
The reproducer don't model timeouts so tests that rely on them end up
with unexpected packets during replay. Skip them until we can handle
this scenario.
Although it's not entirely clear to me why, this test was generating its
binary in the source directory instead of the build directory. This
patch fixes that following the same approach as other tests.
The purpose of the LLDB_RECORD_DUMMY macro is twofold: it is used in
functions that take arguments that we don't know how to serialize (e.g.
void*) and it's used by function where we want to avoid doing excessive
work because they can be called from a signal handler (e.g.
setTerminalWidth).
To support the latter case, I've disabled API logging form the Recorder
ctor used by the DUMMY macro. This ensures we don't allocate memory when
called from a signal handler.
Summary:
It turns out that the order in which we provide completions for expressions is
nondeterministic. This leads to confusing user experience and also breaks the
reproducer tests (as two LLDB tests can go out of sync due to the
non-determinism in the completion lists)
The reason for the non-determinism is that the CompletionConsumer informs us
about decls in the order in which it finds declarations in the lookup store of
the DeclContexts it visits (mainly this snippet in SemaLookup.cpp):
``` lang=c++
// Enumerate all of the results in this context.
for (DeclContextLookupResult R :
Load ? Ctx->lookups()
: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
[...]
```
This storage of the lookup is sorted by pointer values (see the hash of
`DeclarationName`) and can therefore be non-deterministic. The LLDB code
completion consumer that receives these calls originally expected that the order
of declarations is defined by Clang, but it seems the API expects the client to
provide an order to the completions.
This patch fixes the issue as follows:
* We sort the completions we get from Clang alphabetically and also by the
priority value we get from Clang (with priority value sorting having precedence
over the alphabetical sorting)
* We make all the functions/variables that touch a completion before the sorting
const-qualified. The idea is that this should prevent that we never have
observable side-effect from touching these declarations in a non-deterministic
order (e.g., we don't try to complete the type by accident).
This way we behave like the other parts of Clang which also sort the results by
some deterministic value (usually the name or something computed from a name,
e.g., edit distance to a given string).
We most likely also need to fix the Clang code to make the loop I listed above
deterministic to prevent these issues in the future (tracked in rdar://63442513
). This wouldn't replace the functionality provided in this patch though as we
would still need the priority and overall alphabetical sorting.
Note: I had to increase the lldb-vscode completion limit to 100 as the tests
look for strings that aren't in the first 50 results anymore due to variable
names starting with letters like 'v' (which are now always shown much further
down in the list due to the alphabetical sorting).
Fixes rdar://63200995
Reviewers: JDevlieghere, clayborg
Reviewed By: JDevlieghere
Subscribers: mgrang, abidh
Differential Revision: https://reviews.llvm.org/D80292
Summary: `CommandObject::CheckRequirements` requires cleaning up `m_exe_ctx`
between commands. Function `HandleOptionCompletion` returns without cleaning up
`m_exe_ctx` could cause assert failure in later `CheckRequirements`.
Reviewers: teemperor, JDevlieghere
Reviewed By: teemperor
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D80447
Summary:
For ObjCInterfaceDecls, LLDB iterates over the `methods` of the interface in FindExternalVisibleDeclsByName
since commit ef423a3ba5 .
However, when LLDB calls `oid->methods()` in that function, Clang will pull in all declarations in the current
DeclContext from the current ExternalASTSource (which is again, `ClangExternalASTSourceCallbacks`). The
reason for that is that `methods()` is just a wrapper for `decls()` which is supposed to provide a list of *all*
(both currently loaded and external) decls in the DeclContext.
However, `ClangExternalASTSourceCallbacks::FindExternalLexicalDecls` doesn't implement support for ObjCInterfaceDecl,
so we don't actually add any declarations and just mark the ObjCInterfaceDecl as having no ExternalLexicalStorage.
As LLDB uses the ExternalLexicalStorage to see if it can complete a type with the ExternalASTSource, this causes
that LLDB thinks our class can't be completed any further by the ExternalASTSource
and will from on no longer make any CompleteType/FindExternalLexicalDecls calls to that decl. This essentially
renders those types unusable in the expression parser as they will always be considered incomplete.
This patch just changes the call to `methods` (which is just a `decls()` wrapper), to some ad-hoc `noload_methods`
call which is wrapping `noload_decls()`. `noload_decls()` won't trigger any calls to the ExternalASTSource, so
this prevents that ExternalLexicalStorage will be set to false.
The test for this is just adding a method to an ObjC interface. Before this patch, this unset the ExternalLexicalStorage
flag and put the interface into the state described above.
In a normal user session this situation was triggered by setting a breakpoint in a method of some ObjC class. This
caused LLDB to create the MethodDecl for that specific method and put it into the the ObjCInterfaceDecl.
Also `ObjCLanguageRuntime::LookupInCompleteClassCache` needs to be unable to resolve the type do
an actual definition when the breakpoint is set (I'm not sure how exactly this can happen, but we just
found no Type instance that had the `TypePayloadClang::IsCompleteObjCClass` flag set in its payload in
the situation where this happens. This however doesn't seem to be a regression as logic wasn't changed
from what I can see).
The module-ownership.mm test had to be changed as the only reason why the ObjC interface in that test had
it's ExternalLexicalStorage flag set to false was because of this unintended side effect. What actually happens
in the test is that ExternalLexicalStorage is first set to false in `DWARFASTParserClang::CompleteTypeFromDWARF`
when we try to complete the `SomeClass` interface, but is then the flag is set back to true once we add
the last ivar of `SomeClass` (see `SetMemberOwningModule` in `TypeSystemClang.cpp` which is called
when we add the ivar). I'll fix the code for that in a follow-up patch.
I think some of the code here needs some rethinking. LLDB and Clang shouldn't infer anything about the ExternalASTSource
and its ability to complete the current type form the `ExternalLexicalStorage` flag. We probably should
also actually provide any declarations when we get asked for the lexical decls of an ObjCInterfaceDecl. But both of those
changes are bigger (and most likely would cause us to eagerly complete more types), so those will be follow up patches
and this patch just brings us back to the state before commit ef423a3ba5 .
Fixes rdar://63584164
Reviewers: aprantl, friss, shafik
Reviewed By: aprantl, shafik
Subscribers: arphaman, abidh, JDevlieghere
Differential Revision: https://reviews.llvm.org/D80556
Summary:
Objective-C names are stored in m_demangled, not in m_mangled. The
method in the condition will never return true.
Differential Revision: https://reviews.llvm.org/D79823
Summary:
A struct argument can be passed-by-value to a callee via a pointer to a
temporary stack copy. Add support for emitting an entry value DBG_VALUE
when an indirect parameter DBG_VALUE becomes unavailable. This is done
by omitting DW_OP_stack_value from the entry value expression, to make
the expression describe the location of an object.
rdar://63373691
Reviewers: djtodoro, aprantl, dstenb
Subscribers: hiraditya, lldb-commits, llvm-commits
Tags: #lldb, #llvm
Differential Revision: https://reviews.llvm.org/D80345
Don't run tests that use address sanitizer inside an address-sanitized
LLDB. The tests don't support that configuration. Incidentally they
were skipped on green dragon for a different reason, so this hasn't
come up there before.
Many tests use (commented out) print statement for debugging the test
itself. This patch adds a new trace method to lldbtest to reuse the
existing tracing infrastructure and replace these print statements.
Differential revision: https://reviews.llvm.org/D80448
The llvm DWARFExpression dump is nearly identical, but better -- for
example it does print a spurious space after zero-argument expressions.
Some parts of our code (variable locations) have been already switched
to llvm-based expression dumping. This switches the remainder: unwind
plans and some unit tests.
Summary:
This is an attempt to fix https://bugs.llvm.org/show_bug.cgi?id=45988,
where SBValue::GetNumChildren returns 2, but SBValue::GetChildAtIndex(1) returns
an invalid value sentinel.
The root cause of this seems to be that GetNumChildren can return the number of
children of a wrong value. In particular, for pointers GetNumChildren just
recursively calls itself on the pointee type, so it effectively walks chains of
pointers. This is different from the logic of GetChildAtIndex, which only
recurses if pointee.IsAggregateType() returns true (IsAggregateType is false for
pointers and references), so it never follows chain of pointers.
This patch aims to make GetNumChildren (more) consistent with GetChildAtIndex by
only recursively calling GetNumChildren for aggregate types.
Ideally, GetNumChildren and GetChildAtIndex would share the code that decides
which pointers/references are followed, but that is a bit more invasive change.
Reviewers: teemperor, jingham, clayborg
Reviewed By: teemperor, clayborg
Subscribers: clayborg, labath, shafik, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D80254
This patchs adds an optional warning that is printed when stopped at a
frame that was compiled in a source language that LLDB has no plugin
for.
The motivational use-case is debugging Swift code on Linux. When the
user accidentally invokes the system LLDB that was built without the
Swift plugin, it is very much non-obvious why debugging doesnt
work. This warning makes it easy to figure out what went wrong.
<rdar://problem/56986569>
This reverts commit 5f88f39ab8. It broke these
three tests on the Window bot:
lldb-api :: commands/expression/completion/TestExprCompletion.py
lldb-api :: lang/cpp/scope/TestCppScope.py
lldb-api :: lang/cpp/standards/cpp11/TestCPP11Standard.py
These files haven't been touched since 2015. According to Pavel these
were intended to be test for the test framework which never really took
of and are mostly irrelevant by now.
Differential revision: https://reviews.llvm.org/D80408
200 microseconds is not enough time for any expression to execute
reliably. On linux, calling pthread_exit can result in call to dlopen,
which cannot complete in that time, particularly when running under a
debugger.
On linux, this test failed all the time, on macos, about two thirds of
runs were failing. This patch increases the timeout to 100ms, which is
enough to get it passing reliably on linux, though I wouldn't be
surprised if an even bigger timeout would be needed for remote test
runs.
Summary:
Currently we never enable C++14 in the expression evaluator. This enables it when the language of the program is C++14.
It seems C++17 and so on isn't yet in any of the language enums (and the DWARF standard it seems), so C++17 support will be a follow up patch.
Reviewers: labath, JDevlieghere
Reviewed By: labath, JDevlieghere
Subscribers: aprantl
Differential Revision: https://reviews.llvm.org/D80308
Summary: We are not doing this very often, but sometimes it's convenient when I can just << ConstStrings into llvm::errs() during testing.
Reviewers: labath, JDevlieghere
Reviewed By: labath, JDevlieghere
Subscribers: JDevlieghere
Differential Revision: https://reviews.llvm.org/D80310
Remove commented out code, fix the indentation and always use the full
path to the executable. The latter is necessary for the test to pass
from reproducer replay.
Jonas Devlieghere