This reverts commit fd0ab3b3eb.
The fix here is incorrect and the actual fault was an incorrect test Makefile.
To give some more background:
The original test for D80798 compiled three source files into either one
executable or one executable + 2 shared libraries, each being one different
test setup. If both the monolithic executable and the shared libraries
where compiled in the same directory, then Make would overwrite the .o files
of one test setup with the other. This caused that while -fPIC was passed
correctly to the test setup with the shared libraries, the compiler invocations
for the monolithic executable would later overwrite these object files (and
as only the test setup with the shared library used -fPIC, it appeared as if
the shared library object files didn't receive the -fPIC flag).
Thanks to Pavel for figuring this out.
Summary:
It seems that when we rewrite a few rules to only build a dylib (i.e., when DYLIB_ONLY is set),
the rule for setting the CFLAGS for the dylib's object file compilation will no longer work. From what I can
see this is because in DYLIB_ONLY mode we pretend to compile the main executable so
the DYLIB_OBJECTS scope is actually never used.
This patch makes `-fPIC` unstopped if DYLIB_ONLY is set so that -fPIC actually ends up in the
CFLAGS for the dylib object file compilation.
The test for this is D80798 which only compiles on Linux with this patch.
Reviewers: friss, labath
Reviewed By: friss
Subscribers: JDevlieghere
Differential Revision: https://reviews.llvm.org/D80789
We are leaking forked processes on macOS because the cleanup function
was checking the existence of /proc/pid which does not exist on macOS.
I've changed the code to be platform agnostic.
Don't run tests that use undefined behavior 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.
Don't run tests that use thread 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.
Summary:
This patch adds two new arguments to the MakeInlineTest function. The
main motivation is a follow-up patch I'm preparing, but they seem
generally useful.
The first argument allows the user to specify the "build dictionary".
With this argument one can avoid the need to provide a custom Makefile
if all he needs is to override a couple of make variables. This hooks in
neatly into the existing dictionary support for non-inline tests.
The second argument specifies the name of the test. This could be used
to provide better names to the generated test classes, but it's mainly
useful in conjuction with the first argument: now that we can specify a
custom build dictionary, it may sometimes make sense to run the same
test twice with different build configurations. To achieve that, we need
to give the two tests different names, and this argument achieves that.
The usage of the arguments is demonstrated via TestBasicEntryValues.py.
Reviewers: vsk, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D80518
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
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
Summary: Adding this in line with "stopCommands" and "exitCommands" so that we can run commands at the end of the debugging session.
Reviewers: clayborg, wallace, labath
Reviewed By: clayborg, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D79726
Skip tests or parts thereof that aren't expected to work when run from a
reproducer. Also improve the doc comments in configuration.py to prevent
mistakes in the future.
The reproducers' working directory is set to the current working
directory when they are initialized. While this is not optimal, as the
cwd can change during a debug session, it has been sufficient so far.
The current approach doesn't work for the API test suite however because
dotest temporarily changes the directory to where the test's Python file
lives.
This patch adds an API to tell the reproducers what to set the CWD to.
This is a NO-OP in every mode but capture.
Differential revision: https://reviews.llvm.org/D79825
Summary:
The test machinery translates each continuous block of "//%" comments
into a single breakpoint. If there's no code between the blocks the
breakpoints will end up at the same location in the program. When the
process stops at a breakpoint lldb correctly reports all breakpoint IDs,
but the test machinery only looks at the first one. This results in a
very dangerous situation as it means some checks can be silently
stopped.
This patch fixes that by making the test machinery iterate through all
breakpoints at a given location and execute all commands.
Reviewers: vsk, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D79563
Because LLDB isn't the one spawning the subprocess, the PID is different
during replay. Exclude it form the substring check during replay.
Depends on D79646 to pass with reproducer replay.
Summary:
Currently loading core files on lldb-vscode is broken because there's a check in the attach workflow that asserts that the PID is valid, which of course fails for this case.
Hence, I'm adding a "coreFile" argument for the attach request, which does the work correctly.
I don't know how to test it effectively so that it runs on the buildbots and the debugger can in fact makes sense of it. Anyway, the change has been relatively simple.
Reviewers: labath, clayborg
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D78839
[intel-pt] Improve the way the test determines whether to run
- Now I'm creating a default value for the new test parameter
- I fixed a small mistake in the skipping logic of the test
... I forgot to clear the cmake cache when testing my diff
Summary:
@labath raised a concern on the way I was skipping this test. I think that was
fair and I found a better way.
Now I'm skipping if the CMAKE flag LLDB_BUILD_INTEL_PT is false.
I added an enabled_plugins entry in the dotest configuration, which gets
set by lit or lldb-dotest. The only available plugin right now is
'intel-pt', but I imagine it will be useful in the future for other
kinds of plugins that get determined at configuration time. I didn't
want to add a new argument option --enable-intel-pt or something or the
sort, as it wouldn't be useful for other cases.
Reviewers: labath, clayborg
Subscribers: lldb-commits, labath
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77452
Make it possible to capture reproducers from the API test suite. Given
the symmetry between capture and replay, this patch also adds the
necessary code for replay. For now this is a NO-OP until the
corresponding reproducer instrumentation changes land.
For more info please refer to the RFC on lldb-dev:
http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html
Differential revision: https://reviews.llvm.org/D77588
Summary:
This patch introduces a header "dylib.h" which can be used in tests to
handle shared libraries semi-portably. The shared library APIs on
windows and posix systems look very different, but their underlying
functionality is relatively similar, so the mapping is not difficult.
It also introduces two new macros to wrap the functinality necessary to
export/import function across the dll boundary on windows. Previously we
had the LLDB_TEST_API macro for this purpose, which automagically
changed meaning depending on whether we were building the shared library
or the executable. While convenient for simple cases, this approach was
not sufficient for the more complicated setups where one deals with
multiple shared libraries.
Lastly it rewrites TestLoadUnload, to make use of the new APIs. The
trickiest aspect there is the handling of DYLD_LIBRARY_PATH on macos --
previously setting this variable was not needed as the test used
@executable_path-relative dlopens, but the new generic api does not
support that. Other systems do not support such dlopens either so the
test already contained support for setting the appropriate path
variable, and this patch just makes that logic more generic. In doesn't
seem that the purpose of this test was to exercise @executable_path
imports, so this should not be a problem.
These changes are sufficient to make some of the TestLoadUnload tests
pass on windows. Two other tests will start to pass once D77287 lands.
Reviewers: amccarth, jingham, JDevlieghere, compnerd
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77662
Using the approach suggested by Pavel in D77588, this patch introduces a
new lldbconfig module that lives next to the lldb module. It makes it
possible to make the lldb module configurable before importing it. More
specifically it makes it possible to delay initializing the debugger,
which is needed for testing the reproducer.
Differential revision: https://reviews.llvm.org/D77661
Summary:
When using source maps for a breakpoint, in order to find the actual source that breakpoint has resolved, we
need to use a query similar to what CommandObjectSource::DumpLinesInSymbolContexts does, which is the logic
used by the CLI to display the source line at a given breakpoint. That's necessary because from a breakpoint
location you only have an address, which points to the original source location, not the source mapped one.
in the setBreakpoints request handler, we haven't been doing such query and we were returning the original
source location, which was breaking the UX of VSCode, as many breakpoints were being set but not displayed
in the source file next to each line. Besides, clicking the source path of a breakpoint in the breakpoints
view in the debug tab was not working for this case, as VSCode was trying to open a non-existent file, thus
showing an error to the user.
Ideally, we should do the query mentioned above to find the actual source location to respond to the IDE,
however, that query is expensive and users can have an arbitrary number of breakpoints set. As a simpler fix,
the request sent by VSCode already contains the full source path, which exists because the user set it from
the IDE itself, so we can simply reuse it instead of querying from the SB API.
I wrote a test for this, and found out that I had to move SetSourceMapFromArguments after RunInitCommands in
lldb-vscode.cpp, because an init command used in all tests is `settings clear -all`, which would clear the
source map unless specified after initCommands. And in any case, I think it makes sense to have initCommands
run before anything the debugger would do.
Reviewers: clayborg, kusmour, labath, aadsm
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76968
Summary:
If we don't have a current frame then we can still run many expressions
as long as we have an active target. With this patch `expect_expr` directly
calls the target's EvaluateExpression function when there is no current frame.
Reviewers: labath
Reviewed By: labath
Subscribers: JDevlieghere
Differential Revision: https://reviews.llvm.org/D77197
Summary:
If no custom launching is used, lldb-vscode launches a program with an empty environment by default. In some scenarios, the user might want to simply use the same environment as the IDE to have a set of working environment variables (e.g. PATH wouldn't be empty). In fact, most DAPs in VSCode have this behavior by default. In other cases the user definitely needs to set their custom environment, which is already supported. To make the first case easier for the user (e.g. not having to copy the PATH to the launch.json every time they want to debug simple programs that rely on PATH), a new option is now offered. inheritEnvironment will launch the program copying its own environment, and it's just a boolean flag.
{F11347695}
Reviewers: clayborg, aadsm, diazhector98, kusmour
Subscribers: labath, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74636
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>
Global properties are shared between debugger instances and
if a test doesn't clear changes in settings it made,
this leads to side effects in other tests.
Differential Revision: https://reviews.llvm.org/D75537
This patch allows skipping a test based on a default setting, which is
useful when running the testsuite in different "modes" based on a
default setting. This is a feature I need for the Swift testsuite, but
I think it's generally useful.
Differential Revision: https://reviews.llvm.org/D75864
A couple of tests sporadically fail on these assertions, but the error
messages do not give a clue as to what has actually happened.
Improve them so that we can better understand what is going wrong.
This patch forces architecture "arm" if underlying os reports core
armv7l or armv8l. On linux systems 32 bit sysroot running on 64bit
AArch64 hardware reports armv7l or armv8l which is essently arm
32bit mode. This fixes 5 testcases on 32bit arm.
Some tests set settings and don't clean them up, this leads to side effects in other tests.
The patch removes a global debugger instance with a per-test debugger to avoid such effects.
From what I see, lldb.DBG was needed to determine the platform before a test is run,
lldb.selected_platform is used for this purpose now. Though, this required adding a new function
to the SBPlatform interface.
Differential Revision: https://reviews.llvm.org/D74903
Summary:
Currently the test suite runs with enabled automatically applied Clang fix-its for expressions.
This is causing that sometimes incorrect expressions in tests are still evaluated even though they
are actually incorrect. Let's disable this feature in the test suite so that we know when expressions
are wrong and leave the fix-it testing to the dedicated tests for that feature.
Also updates the `lang/cpp/operators/` test as it seems Clang needs the `struct` keywords
before C and would otherwise fail without fixits.
Reviewers: jingham, JDevlieghere, shafik
Reviewed By: JDevlieghere, shafik
Subscribers: shafik, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74957
Summary:
Requesting registers one by one takes a while in our project.
We want to get rid of it by using target.xml.
Reviewers: jarin, labath, omjavaid
Reviewed By: labath, omjavaid
Subscribers: omjavaid, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74217
Summary:
Currently when printing data types we include implicit scopes such as inline namespaces or anonymous namespaces.
This leads to command output like this (for `std::set<X>` with X being in an anonymous namespace):
```
(lldb) print my_set
(std::__1::set<(anonymous namespace)::X, std::__1::less<(anonymous namespace)::X>, std::__1::allocator<(anonymous namespace)::X> >) $0 = size=0 {}
```
This patch removes all the implicit scopes when printing type names in TypeSystemClang::GetDisplayTypeName
so that our output now looks like this:
```
(lldb) print my_set
(std::set<X, std::less<X>, std::allocator<X> >) $0 = size=0 {}
```
As previously GetDisplayTypeName and GetTypeName had the same output we actually often used the
two as if they are the same method (they were in fact using the same implementation), so this patch also
fixes the places where we actually want the display type name and not the actual type name.
Note that this doesn't touch the `GetTypeName` class that for example the data formatters use, so this patch
is only changes the way we display types to the user. The full type name can also still be found when passing
'-R' to see the raw output of a variable in case someone is somehow interested in that.
Partly fixes rdar://problem/59292534
Reviewers: shafik, jingham
Reviewed By: shafik
Subscribers: christof, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74478
Raphael Isemann