Summary:
This patch refactors the internal completion API. It now takes (as far as possible) a single
CompletionRequest object instead o half a dozen in/out/in-out parameters. The CompletionRequest
contains a common superset of the different parameters as far as it makes sense. This includes
the raw command line string and raw cursor position, which should make the `expr` command
possible to implement (at least without hacks that reconstruct the command line from the args).
This patch is not intended to change the observable behavior of lldb in any way. It's also as
minimal as possible and doesn't attempt to fix all the problems the API has.
Some Q&A:
Q: Why is this not fixing all the problems in the completion API?
A: Because is a blocker for the expr command completion which I want to get in ASAP. This is the
smallest patch that unblocks the expr completion patch and which allows trivial refactoring in the future.
The patch also doesn't really change the internal information flow in the API, so that hopefully
saves us from ever having to revert and resubmit this humongous patch.
Q: Can we merge all the copy-pasted code in the completion methods
(like computing the current incomplete arg) into CompletionRequest class?
A: Yes, but it's out of scope for this patch.
Q: Why the `word_complete = request.GetWordComplete(); ... ` pattern?
A: I don't want to add a getter that returns a reference to the internal integer. So we have
to use a temporary variable and the Getter/Setter instead. We don't throw exceptions
from what I can tell, so the behavior doesn't change.
Q: Why are we not owning the list of matches?
A: Because that's how the previous API works. But that should be fixed too (in another patch).
Q: Can we make the constructor simpler and compute some of the values from the plain command?
A: I think this works, but I rather want to have this in a follow up commit. Especially when making nested
request it's a bit awkward that the parsed arguments behave as both input/output (as we should in theory
propagate the changes on the nested request back to the parent request if we don't want to change the
behavior too much).
Q: Can't we pass one const request object and then just return another result object instead of mixing
them together in one in/out parameter?
A: It's hard to get keep the same behavior with that pattern, but I think we can also get a nice API with just
a single request object. If we make all input parameters read-only, we have a clear separation between what
is actually an input and what an output parameter (and hopefully we get rid of the in-out parameters).
Q: Can we throw out the 'match' variables that are not implemented according to the comment?
A: We currently just forward them as in the old code to the different methods, even though I think
they are really not used. We can easily remove and readd them once every single completion method just
takes a CompletionRequest, but for now I prefer NFC behavior from the perspective of the API user.
Reviewers: davide, jingham, labath
Reviewed By: jingham
Subscribers: mgorny, friss, lldb-commits
Differential Revision: https://reviews.llvm.org/D48796
llvm-svn: 336146
Summary:
This patch updates a few MI commands using a new way of
handling an errors in lldb-mi and removes unnecessary
m_lldbResult variables.
Reviewers: aprantl, clayborg, labath
Reviewed By: aprantl, clayborg
Subscribers: ki.stfu, lldb-commits
Differential Revision: https://reviews.llvm.org/D47992
llvm-svn: 336009
Summary:
This test makes sure we are able to read the shorter build-ids which are
generated by lld.
To make this work, I've extended lldb-test to print the UUID of the
loaded object file. I've renamed the lldb-test subcommand from
"module-sections" to "object-file" to reflect the fact it prints more
than just the sections.
I've also added the module Architecture to the output, so we could avoid
printing the entire symbol file information just to get the ArchSpec
details in the lc_version_min test (which was also the only test in it's
folder not using the module-sections command).
Reviewers: aprantl, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D48646
llvm-svn: 335967
Summary:
The data structure is optimized for the case where the UUID size is <=
20 bytes (standard length emitted by the GNU linkers), but larger sizes
are also possible.
I've modified the string conversion function to support the new sizes as
well. For standard UUIDs it maintains the traditional formatting
(4-2-2-2-6). If a UUID is shorter, we just cut this sequence short, and
for longer UUIDs it will just repeat the last 6-byte block as long as
necessary.
I've also modified ObjectFileELF to take advantage of the new UUIDs and
avoid manually padding the UUID to 16 bytes. While there, I also made
sure the computed UUID does not depend on host endianness.
Reviewers: clayborg, lemo, sas, davide, espindola
Subscribers: emaste, arichardson, lldb-commits
Differential Revision: https://reviews.llvm.org/D48633
llvm-svn: 335963
The code was creating a StringRef to a temporary std::string. The
solution is to just drop the .str() from the original StringRef.
This manifested it self as the new TestCompletions test failing in some
configurations.
llvm-svn: 335960
Now that we have gmock, we can use its matchers to better express the
test assertions. The main advantage of this is that when things fail,
the test will now print the expected and actual lists of completed
strings instead of just a not-very-helpful "false is not true" message.
llvm-svn: 335955
Summary:
The test fails because we don't rewrite the slash behind `foo` to the OS specific
separator (as the completion API doesn't support this kind of rewriting). However,
we assume that this part of the string is rewritten in the test case, which broke
on Windows.
Reviewers: stella.stamenova
Reviewed By: stella.stamenova
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D48746
llvm-svn: 335934
This provides an efficient (at least on Posix platforms) way to offload to the
target process the search & loading of a library when all we have are the
library name and a set of potential candidate locations.
<rdar://problem/40905971>
llvm-svn: 335912
Summary: On Windows, the newer DIA SDKs end up producing function names that contain the return type as well. This means that the function name returned in the test will contain the return type (int) in addition to the name of the function and the type of the input (a(int)). To account for the possibility of both, the test should pass if the function name matches either pattern.
Reviewers: zturner, asmith
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D48654
llvm-svn: 335906
Summary:
To successfully open a core file, we need to have LLVM built with
support for the relevant target. Right now, if one does not have the
appropriate targets configured, the tests will fail.
This patch uses the GetBuildConfiguration SB API to inform the test (and
anyone else who cares) about the list of supported LLVM targets. The
test then uses this information to approriately skip the tests.
Reviewers: clayborg, jingham
Subscribers: martong, lldb-commits
Differential Revision: https://reviews.llvm.org/D48641
llvm-svn: 335859
Summary:
This patch fixes a problem with retrieving a function symbol by an
address in a nested block. In the current implementation of
ResolveSymbolContext function it retrieves a symbol with
PDB_SymType::None and then checks if found symbol's tag equals to
PDB_SymType::Function. So, if nested block's symbol was found,
ResolveSymbolContext does not resolve a function.
Reviewers: asmith, labath, zturner
Reviewed By: asmith, labath
Differential Revision: https://reviews.llvm.org/D47939
Patch by Aleksandr Urakov <aleksandr.urakov@jetbrains.com>
llvm-svn: 335822
When running the test suite with .debug_names a bunch of tests were
failing because GetCompleteObjCClass was not yet implemented for
DebugNamesDWARFIndex. This patch adds the required logic.
We use the .debug_names to find the Objective-C class and then rely on
DW_AT_APPLE_objc_complete_type to find the complete type. If we can't
find it or the attribute is not supported, we return a list of potential
complete types.
Differential revision: https://reviews.llvm.org/D48596
llvm-svn: 335776
This change allows to make AddressClass strongly typed enum and not to have issues with old versions of SWIG that don't support enum classes.
llvm-svn: 335710
Summary:
During the previous attempt to generalize the UUID class, it was
suggested that we represent invalid UUIDs as length zero (previously, we
used an all-zero UUID for that). This meant that some valid build-ids
could not be represented (it's possible however unlikely that a checksum of
some file would be zero) and complicated adding support for variable
length build-ids (should a 16-byte empty UUID compare equal to a 20-byte
empty UUID?).
This patch resolves these issues by introducing a canonical
representation for an invalid UUID. The slight complication here is that
some clients (MachO) actually use the all-zero notation to mean "no UUID
has been set". To keep this use case working (while making it very
explicit about which construction semantices are wanted), replaced the
UUID constructors and the SetBytes functions with named factory methods.
- "fromData" creates a UUID from the given data, and it treats all bytes
equally.
- "fromOptionalData" first checks the data contents - if all bytes are
zero, it treats this as an invalid/empty UUID.
Reviewers: clayborg, sas, lemo, davide, espindola
Subscribers: emaste, lldb-commits, arichardson
Differential Revision: https://reviews.llvm.org/D48479
llvm-svn: 335612
If we have a function with signature f(addr_t, AddressClass), it is easy to muddle up the order of arguments without any warnings from compiler. 'enum class' prevents passing integer in place of AddressClass and vice versa.
llvm-svn: 335599
This method does one of two things:
1. finds a minimum os deployment version # in a Mach-O load
command and saves the three parts in the m_sdk_version, or
2. finds no valid min os version # load command, pushes a
sentinel value on the m_sdk_version vector so we don't search
the same load commands multiple times.
There was a little bug when we found a load command with
a version of 0.0.0 - the method would not add anything to
the m_sdk_version vector but would declare that a success.
It would not push the sentinel value to the vector.
There was code later in the method which assumed that
the vector always had a sentinel value, at least, and that
code could crash when this method was called back when
evaluating a Swift expression. (these version #'s are
fetched lazily so it wouldn't happen when the object file
was parsed, only when doing an expression that needed
the version #).
<rdar://problem/41372699>
llvm-svn: 335556
pseudo_barrier_wait() begins by decrementing an atomic variable. Since
these are always_inline in libc++, there is no line table anchor to
break on before we decrement it. This meant that on gcc we stopped after
the variable has been decremented, which meant that thread2 could have
exited, violating the test setup. On clang this wasn't a problem
because it generated some line table entries for the do{}while(0) loop
in the macro, so we still ended up stopping, before we touched the
variable.
I fix this by adding a dummy statement before the pseudo_barrier_wait()
command and setting the breakpoint there.
llvm-svn: 335476
This reverts r335432 because remove_dots() is expensive and measuring
its impact showed an observable performance regression
(https://reviews.llvm.org/D45977#1078510).
llvm-svn: 335448
Removing redundant components from the path seems pretty harmless.
Rather than checking whether this is necessary and then actually doing
so, always invoke remove_dots to start with a normalized path.
llvm-svn: 335432
Filenames with test results contain only the class name which makes it more
difficult to find it if the same class name is present in multiple *.py files.
packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
-class ReturnValueTestCase(TestBase):
+class StepAvoidsNoDebugTestCase(TestBase):
as ReturnValueTestCase is already present in:
packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
-class CreateDuringStepTestCase(TestBase):
+class CrashDuringStepTestCase(TestBase):
as CreateDuringStepTestCase is already present in:
packages/Python/lldbsuite/test/functionalities/thread/create_during_step/TestCreateDuringStep.py
packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
-class TestCStepping(TestBase):
+class StepUntilTestCase(TestBase):
as TestCStepping is already present in:
packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py
llvm-svn: 335431
Replaces custom implementations of append and prepend with calls to
llvm's path library. This is part of a series of patches (started in
D48084) to delegate common operations to llvm::sys::path.
llvm-svn: 335430
Fixes include:
- fix all lint errors
- add code that will automatically register and LLDB command classes by detecting the classes and any classes that have a "register_lldb_command" function
- automatically fill in the correct module name when registering commands
- automatically fill in the class name when registering command
llvm-svn: 335401
libstdc++ will soon be dropped from the android NDK. This patch makes
sure we are prepared for that by using libc++ in tests by default (i.e.,
except for libstdc++ data formatter tests).
Only a couple of small tweaks were needed to make this work:
- Add the libc++ include paths to CXXFLAGS only. This was necessary to
make the tests compile with -fmodules. The modules tests have been
disabled, but this way, they will be ready for them if they are
enabled.
- in one test I had to add an explicit std::string copy to make sure the
copy constructor is there for the expression evaluator to find it.
llvm-svn: 335344
Our DWARF parsing code had a workaorund for Objective-C "self" not
being marked as artifial by the compiler. Clang has been doing this
since 2010, so let's just drop the workaround.
llvm-svn: 335313
Jim pointed out that XCode has build configurations that build without
python and removing the ifdefs around the python code breaks them.
This reverts the #ifdef part of the above patch, while keeping the cmake
parts.
llvm-svn: 335260
I've made the code accept only 16 byte UUIDs, which is technically not
NFC (previously it would also accept 20 byte ones, but use only the
first 16 bytes), but this should be more correct as mac UUIDs are always
16 byte long.
llvm-svn: 335247
Instead of a separate GetBytes + GetByteSize methods I introduce a
single GetBytes method returning an ArrayRef.
This is NFC cleanup now, but it should make handling arbitrarily-sized
UUIDs cleaner, should we choose to go that way. I also took the
opportunity to add some unit tests for this class.
llvm-svn: 335244
Instead of #ifdef-ing the contents of all files in the plugin for all
non-python builds, just disable the plugin at the cmake level. Also,
remove spurious extra linking of the Python plugin in liblldb. This
plugin is already included as a part of LLDB_ALL_PLUGINS variable.
llvm-svn: 335236
These tests are extremely environment-dependent. if the environment is
not module-enabled (which is the likely scenario), they won't test
anything. If one happens to have a module-enabled libc++, then the he
will start running into problems.
The first one is that the debug info in pcm file contains relocations
that ObjectFileELF doesn't handle (particularly on non-x86
architectures), but even after that is resolved, it seems we still are
unable to pull debug info out of the pcm file. I've filed pr37893 to
track that, and I am disabling gmodules tests on linux until these
issues are resolved.
llvm-svn: 335235
In a modules build, android is very picky about which symbols are
visible after including libc++ headers (e.g. <cstdio> defines only
std::printf and not ::printf).
This consolidates the tests where this was an issue to always include
the <c???> version of the headers and prefixes the symbols with std:: as
necessary.
Apart from that, there is no functional change in the tests.
llvm-svn: 335149