This patch adds NativeRegisterContext_arm64 ptrace routines to access
AArch64 SVE register set. This patch also adds a test-case to test
AArch64 SVE register access and dynamic size configuration capability.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D79699
I intentionally decided not to reset the column automatically
anywhere, because I don't know where and if at all that should happen.
There should be always an indication of being scrolled (too much)
to the right, so I'll leave this to whoever has an opinion.
Differential Revision: https://reviews.llvm.org/D85290
Rename the existing expectedFailure to expectedFailureIfFn to better
describe its purpose and provide an overload for
unittest2.expectedFailure in decorators.py.
This is the error message from the OS, so we shouldn't check against the
OS-specific part of the string.
Fixes the test on Windows which returns a different error message.
We didn't do anything with the llvm::Error we get from `Open`, so when we end up in the
error case we just crash due to the llvm::Error sanity check. Also add the missing newline
behind the error message so it no longer messes with the next (lldb) prompt.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D85970
This is relanding D81001. The patch originally failed as on newer editline
versions it seems CC_REFRESH will move the cursor to the start of the line via
\r and then back to the original position. On older editline versions like
the one used by default on macOS, CC_REFRESH doesn't move the cursor at all.
As the patch changed the way we handle tab completion (previously we did
REDISPLAY but now we're doing CC_REFRESH), this caused a few completion tests
to receive this unexpected cursor movement in the output stream.
This patch updates those tests to also accept output that contains the specific
cursor movement commands (\r and then \x1b[XC). lldbpexpect.py received an
utility method for generating the cursor movement escape sequence.
Original summary:
I implemented autosuggestion if there is one possible suggestion.
I set the keybinds for every character. When a character is typed, Editline::TypedCharacter is called.
Then, autosuggestion part is displayed in gray, and you can actually input by typing C-k.
Editline::Autosuggest is a function for finding completion, and it is like Editline::TabCommand now, but I will add more features to it.
Testing does not work well in my environment, so I can't confirm that it goes well, sorry. I am dealing with it now.
Reviewed By: teemperor, JDevlieghere, #lldb
Differential Revision: https://reviews.llvm.org/D81001
When LLDB sees only one possible completion for an input, it will add a trailing
space to the completion to signal that to the user. If the current argument is
quoted, that also means LLDB needs to add the trailing quote to finish the
current argument first.
In case the user is in a function with only one local variable and is currently
editing an empty line in the multiline expression editor, then we are in the
unique situation where we can have a unique completion for an empty input line.
(In a normal LLDB session this would never occur as empty input would just list
all the possible commands).
In this special situation our check if the current argument needs to receive a
trailing quote will crash LLDB as there is no current argument and the
completion code just unconditionally tries to access the current argument. This
just adds the missing check if we even have a current argument before we check
if we need to add a terminating quote character.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D85903
Without this, sources with long lines or variable names may overwrite
panel frames, or even overrun to the following line. There's currently
no way to scroll left/right in the views, so that should be added
to handle these cases.
This commit includes fixing constness of some Window functions,
and also makes PutCStringTruncated() consistent with the added
printf-like variant to take the padding as the first argument (can't
add it after the format to the printf-like function).
Differential Revision: https://reviews.llvm.org/D85123
This patch modifies the skipIfRemote decorator so it can apply to a
whole class, which allows us to skip all PExpect tests as a whole.
Differential revision: https://reviews.llvm.org/D85365
Between the time it was created and it was pushed upstream,
99451b4453 has moved the existing
gui gui tests to lldb/test, so move this one too.
And update it to contain TestGuiBasic.py changes since the time
when it was based on that test.
Differential Revision: https://reviews.llvm.org/D85106
lldb-platform contains a very minimal support for the qfProcessInfo
packet, only allowing the simplest query to get most of the testsuite
running, and returning very little information about the matched
processes.
Currently, `target create` has no --platform option. However,
TargetList::CreateTargetInternal which is called under the hood, will
return an error when either no platform or multiple matching platforms
are found, saying that a platform should be specified with --platform.
This patch adds the platform option, but that doesn't solve either of
these errors.
- If more than one platform matches, specifying the platform isn't
going to fix that. The current code will only look at the
architecture instead. I've updated the error message to ask the user
to specify an architecture.
- If no architecture is found, specifying a new one via platform isn't
going to change that either because we already try to find one that
matches the given architecture.
Differential revision: https://reviews.llvm.org/D84809
Summary:
Frame recognizers are stored alongside a flag that indicates whether they were
deleted by the user. If the flag is set, they are supposed to be ignored by the
rest of the frame recognizer code. 'frame recognizer delete' is supposed to set
that flag. 'frame recognizer clear' however actually deletes all frame
recognizers (so, it doesn't set the flag but directly deletes them from the
list).
The current implementation of this concept is pretty broken. `frame recognizer
delete` sets the flag, but it somehow thinks that the recognizer id is an index
in the recognizer list. That's not true as it's actually just a member of each
recognizer entry. So it actually just sets the `deleted` flag for a random other
recognizer. The tests for the recognizer still pass as `frame recognizer list`
is also broken and just completely ignored the `deleted` flag and lists all
recognizers. Also `frame recognizer delete` just ignores if it can't actually
delete a recognizer if the id is invalid.
I think we can simplify this whole thing by just actually deleting recognizers
instead of making sure all code is actually respecting the `deleted` flag. I
assume the intention of this was to make sure that all recognizers are getting
unique ids over the course of an LLDB session, but as `clear` is actually
deleting them and we keep recycling ids, that didn't really work to begin with.
This patch deletes the `deleted` flag and just actually deletes the stored
recognizer. Also adds the missing error message in case it find a recognizer
with a given id.
Reviewers: mib
Reviewed By: mib
Subscribers: abidh, JDevlieghere
Differential Revision: https://reviews.llvm.org/D84404
This patch moves the `history` subcommand from the `command` to `session`
command. I think it makes more sense to have it there because as the `command`
usage suggests, it should be used to manage custom LLDB commands.
However, `history` is essentially tied to a debugging session and holds
all the commands (not specifically custom ones).
This also makes it more discoverable by adding an alias for it (mimicking
the shell builtin).
Differential Revision: https://reviews.llvm.org/D84307
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch introduce a new feature that allows the users to save their
debugging session's transcript (commands + outputs) to a file.
It differs from the reproducers since it doesn't require to capture a
session preemptively and replay the reproducer file in lldb.
The user can choose the save its session manually using the session save
command or automatically by setting the interpreter.save-session-on-quit
on their init file.
To do so, the patch adds a Stream object to the CommandInterpreter that
will hold the input command from the IOHandler and the CommandReturnObject
output and error. This way, that stream object accumulates passively all
the interactions throughout the session and will save them to disk on demand.
The user can specify a file path where the session's transcript will be
saved. However, it is optional, and when it is not provided, lldb will
create a temporary file name according to the session date and time.
rdar://63347792
Differential Revision: https://reviews.llvm.org/D82155
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
registerSharedLibrariesWithTarget was setting the library path
environment variable to the process build directory, but the function is
also accepting libraries in other directories (in which case they won't
be found automatically).
This patch makes the function set the path variable correctly for these
libraries too. This enables us to remove the code for setting the path
variable in TestWeakSymbols.py, which was working only accidentally --
it was relying on the fact that
launch_info.SetEnvironmentEntries(..., append=True)
would not overwrite the path variable it has set, but that is going to
change with D83306.
Reviewers: davide, jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D83552
Summary:
Currently the frame recognizers are stored in a global list (the list in the
StackFrameRecognizersManagerImpl singleton to be precise). All commands and
plugins that modify the list are just modifying that global list of recognizers
which is shared by all Target and Debugger instances.
This is clearly against the idea of LLDB being usable as a library and it also
leads to some very obscure errors as now multiple tests are sharing the used
frame recognizers. For example D83400 is currently failing as it reorders some
test_ functions which permanently changes the frame recognizers of all
debuggers/targets. As all frame recognizers are also initialized in a 'once'
guard, it's also impossible to every restore back the original frame recognizers
once they are deleted in a process.
This patch just moves the frame recognizers into the current target. This seems
the way everyone assumes the system works as for example the assert frame
recognizers is using the current target to find the function/so-name to look for
(which only works if the recognizers are stored in the target).
Reviewers: jingham, mib
Reviewed By: jingham, mib
Subscribers: MrHate, JDevlieghere
Differential Revision: https://reviews.llvm.org/D83757
Always clean up subprocesses on tear down instead of relying on the
caller to do so. This is not only less error prone but also means the
tests can be more concise.
Differential revision: https://reviews.llvm.org/D83787
Summary:
The Scalar class claims to follow the C type conversion rules. This is
true for the Promote function, but it is not true for the implicit
conversions done in the getter methods.
These functions had a subtle bug: when extending the type, they used the
signedness of the *target* type in order to determine whether to do
sign-extension or zero-extension. This is not how things work in C,
which uses the signedness of the *source* type. I.e., C does
(sign-)extension before it does signed->unsigned conversion, and not the
other way around.
This means that: (unsigned long)(int)-1
is equal to (unsigned long)0xffffffffffffffff
and not (unsigned long)0x00000000ffffffff
Unsurprisingly, we have accumulated code which depended on this
inconsistent behavior. It mainly manifested itself as code calling
"ULongLong/SLongLong" as a way to get the value of the Scalar object in
a primitive type that is "large enough". Previously, the ULongLong
conversion did not do sign-extension, but now it does.
This patch makes the Scalar getters consistent with the declared
semantics, and fixes the couple of call sites that were using it
incorrectly.
Reviewers: teemperor, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D82772
Summary:
This replaces the current use of LLDB's own `StringConvert` with LLVM's
`to_integer` which has a less error-prone API and doesn't use special 'error
values' to designate parsing problems.
Where needed I also added missing error handling code that prints a parsing
error instead of continuing with the error value returned from `StringConvert`
(which either gave a cryptic error message or just took the error value
performed an incorrect action with it. For example, `frame recognizer delete -1`
just deleted the frame recognizer at index 0).
Reviewers: #lldb, labath
Reviewed By: labath
Subscribers: labath, abidh, JDevlieghere
Differential Revision: https://reviews.llvm.org/D82297
Summary:
A lot of our tests do 'self.assertTrue(error.Success()'. The problem
with that is that when this fails, it produces a completely useless
error message (False is not True) and the most important piece of
information -- the actual error message -- is completely hidden.
Sometimes we mitigate that by including the error message in the "msg"
argument, but this has two additional problems:
- as the msg argument is evaluated unconditionally, one needs to be
careful to not trigger an exception when the operation was actually
successful.
- it requires more typing, which means we often don't do it
assertSuccess solves these problems by taking the entire SBError object
as an argument. If the operation was unsuccessful, it can format a
reasonable error message itself. The function still accepts a "msg"
argument, which can include any additional context, but this context now
does not need to include the error message.
To demonstrate usage, I replace a number of existing assertTrue
assertions with the new function. As this process is not easily
automatable, I have just manually updated a representative sample. In
some cases, I did not update the code to use assertSuccess, but I went
for even higher-level assertion apis (runCmd, expect_expr), as these are
even shorter, and can produce even better failure messages.
Reviewers: teemperor, JDevlieghere
Subscribers: arphaman, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D82759
These tests are flaky on the reproducer bot. I suspect it has something
to do with the module cache. Skipping the whole category while I
investigate the issue.
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.
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
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
This patch introduces the `(-h|--host)` option to the `platform shell`
command. It allows the user to run shell commands from the host platform
(always available) without putting lldb in the background.
Since the default behaviour of `platform shell` is to run the command of
the selected platform, having such a choice can be quite handy when
debugging remote targets, for instances.
This patch also introduces a `shell` alias, to improve the command
discoverability and make it more convenient to use for the user.
rdar://62856024
Differential Revision: https://reviews.llvm.org/D79659
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
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.
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.
This skips some tests that pass with active replay (which doesn't check
the output) but fail with passive replay. Valid reasons for this
include:
- Checking the output of the process (which doesn't run during replay),
- Checking files that cannot be captured in the VFS (non-existing or
unreadable files or files that are removed during test),
Unfortunately there's no good way to mark a test as supported for active
replay but unsupported for passive replay because the number and order
of API calls needs to be identical during capture and replay. I don't
think this is a huge loss however.
Summary:
Sometimes users think that setting a function regex for all function that contain the word 'needle' in their
name looks like this: `*needle*`. However, LLDB only searches the function name and doesn't fully match
it against the regex, so the leading and trailing '*' operators don't do anything and actually just cause the
regex engine to reject the regular expression with "repetition-operator operand invalid".
This patch makes this a bit more obvious to the user by printing a warning that a leading '*' before this
regular expression here doesn't have any purpose (and will cause an error). This doesn't attempt to detect
a case where there is only a trailing '*' as that would involve parsing the regex and it seems the most
common way to end up in this situation is by doing `rbreak *needle*`.
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D78809
This patch fixes the test failure happening on Windows introduced by
`015117411e11458f9816ba4359246132164a4297`.
Since the failure message comes from the OS, the test needs to support both
UNIX and Windows messages.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
When trying to read a core file that is not owned by the user running lldb
and that doesn't have read permission on the file, lldb shows a misleading
error message:
```
Unable to find process plug-in for core file
```
This is due to the fact that currently, lldb doesn't check the file
ownership. And when trying to to open and read a core file, the syscall
fails, which prevents a process to be created.
Since lldb already have a portable `open` syscall interface, lets take
advantage of that and delegate the error handling to the syscall
itself. This way, no matter if the file exists or if the user has proper
ownership, lldb will always try to open the file, and behave accordingly
to the error code returned.
rdar://42630030
https://reviews.llvm.org/D78712
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
Currently the breakpoint command is prompting the user to file a bug report if the provided regex is invalid:
```
(lldb) rbreak *foo
error: Function name regular expression could not be compiled: "Inconvertible error value. An error has occurred that could not be converted to a known std::error_code. Please file a bug. repetition-operator operand invalid"
```
The reason is simply that we are using the wrong StringError constructor (the one with the error code as the first parameter
is also printing the string version of the error code, and the inconvertible error code is just an invalid place holder code with
that description). Switching the StringError constructor parameters will only print the error message we get from the regex
engine when we convert the error into a string.
I checked the rest of the code base and I couldn't find the same issue anywhere else.
Fixes rdar://62233561
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D78808
In ImportContext(…) we may call into CompleteDecl(…) which if FromRecrord is not
defined will start the definition of a ToRecord but from what I can tell at least
one of the paths though here don't ensure we complete the definition.
For a RecordDecl this can be problematic since this means we won’t import base
classes and we won’t have any of the methods or types we inherit from these bases.
Differential Revision: https://reviews.llvm.org/D78000
Add the skipIfReproducer decorator to the remaining tests that fail to
replay because the GDB remote packets diverge during replay. This is
*not* expected and should be fixed, but figuring out exactly what caused
the divergence has proven pretty difficult to track down.
I've marked these tests as skipped for now so we can get clean results
and detect new regressions. I have no evidence to believe that these
failures have the same root cause, so I've not assigned them a PR.
Summary:
...and replace it with m_last_file_spec instead.
When Source Cache is enabled, the value stored in m_last_file_sp is
already in the Source Cache, and caching it again in SourceManager
brings no extra benefit. All we need is to "remember" the last used
file, and FileSpec can serve the same purpose.
When Source Cache is disabled, the user explicitly requested no caching
of source files, and therefore, m_last_file_sp should NOT be used.
Bug: llvm.org/PR45310
Depends on D76805.
Reviewers: labath, jingham
Reviewed By: jingham
Subscribers: labath, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76806
Since D77214 there is a testsuite regression for TestFixIts.py
on Fedora 31 x86_64.
File "/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/commands/expression/fixits/TestFixIts.py", line 148, in test_with_target
self.assertEquals(value.GetError().GetCString(), "error: No value")
AssertionError: 'error: error: Multiple internal symbols found for \'d\'\nid = {0x00000d2a}, ran [truncated]... != 'error: No value'
That is because Fedora glibc incl. libm.so contains also ELF debug
symbols and there exists a 'd' symbol:
(gdb) p d
$1 = {i = {0, 1076887552}, d = 16}
(gdb) p &d
$2 = (const number *) 0x7ffff78e8bc0 <d>
(gdb) info sym 0x7ffff78e8bc0
d in section .rodata of /lib64/libm.so.6
$ nm /lib64/libm.so.6 |grep ' d$'
00000000000bfbc0 r d
00000000000caa20 r d
00000000000caa20 r d
00000000000caa20 r d
glibc-build$ for i in `find -name "*.o"`;do nm 2>/dev/null $i|grep ' d$' && echo $i;done
0000000000000080 r d
./math/s_atan-fma4.o
0000000000000080 r d
./math/s_atan-avx.o
0000000000000080 r d
./math/s_atan.o
The final function call to `test_X` is failing on aarch64-linux with SIGILL.
Function calls to previous expressions seem to just not work on aarch64-linux
but I don't see another way to test the multiple-run Fix-Its.
This patch refactors the test that the skipIf for aarch64 Linux only covers
the part of the test that was added D77214.
I fixed the bug that the "log timer" has no tab command.
Original code has the only CommandObjectLogTimer class, but it is not
sufficient. Thus I divided the content of CommandObjectLog class into
CommandObjectLogEnable class, CommandObjectLogDisable class,
CommandObjectLogDump class, CommandObjectLogReset class,
CommandObjectLogIncrement class.
Reviewed by: teemperor
Differential Revision: https://reviews.llvm.org/D76906
Summary:
Usually when Clang emits an error Fix-It it does two things. It emits the diagnostic and then it fixes the
currently generated AST to reflect the applied Fix-It. While emitting the diagnostic is easy to implement,
fixing the currently generated AST is often tricky. That causes that some Fix-Its just keep the AST as-is or
abort the parsing process entirely. Once the parser stopped, any Fix-Its for the rest of the expression are
not detected and when the user manually applies the Fix-It, the next expression will just produce a new
Fix-It.
This is often occurring with quickly made Fix-Its that are just used to bridge temporary API changes
and that often are not worth implementing a proper API fixup in addition to the diagnostic. To still
give some kind of reasonable user-experience for users that have these Fix-Its and rely on them to
fix their expressions, this patch adds the ability to retry parsing with applied Fix-Its multiple time to
give the normal Fix-It experience where things Clang knows how to fix are not causing actual expression
error (at least when automatically applying Fix-Its is activated).
The way this is implemented is just by having another setting in the expression options that specify how
often we should try applying Fix-Its and then reparse the expression. The default setting is still 1 for everyone
so this should not affect the speed in which we fail to parse expressions.
Reviewers: jingham, JDevlieghere, friss, shafik
Reviewed By: shafik
Subscribers: shafik, abidh
Differential Revision: https://reviews.llvm.org/D77214
Summary:
LLDB currently applies Fix-Its if they are attached to a Clang diagnostic that has the
severity "error". Fix-Its connected to warnings and other severities are supposed to
be ignored as LLDB doesn't seem to trust Clang Fix-Its in these situations.
However, LLDB also ignores all Fix-Its coming from "note:" diagnostics. These diagnostics
are usually emitted alongside other diagnostics (both warnings and errors), either to keep
a single diagnostic message shorter or because the Fix-It is in a different source line. As they
are technically their own (non-error) diagnostics, we currently are ignoring all Fix-Its associated with them.
For example, this is a possible Clang diagnostic with a Fix-It that is currently ignored:
```
error: <user expression 1>:2:10: too many arguments provided to function-like macro invocation
ToStr(0, {,})
^
<user expression 1>:1:9: macro 'ToStr' defined here
#define ToStr(x) #x
^
<user expression 1>:2:1: cannot use initializer list at the beginning of a macro argument
ToStr(0, {,})
^ ~~~~
```
We also don't store "note:" diagnostics at all, as LLDB's abstraction around the whole diagnostic
concept doesn't have such a concept. The text of "note:" diagnostics is instead
appended to the last non-note diagnostic (which is causing that there is no "note:" text in the
diagnostic above, as all the "note:" diagnostics have been appended to the first "error: ..." text).
This patch fixes the ignored Fix-Its in note-diagnostics by appending them to the last non-note
diagnostic, similar to the way we handle the text in these diagnostics.
Reviewers: JDevlieghere, jingham
Reviewed By: JDevlieghere
Subscribers: abidh
Differential Revision: https://reviews.llvm.org/D77055
This patch was reverted because it introduced a failure in
TestHelloWorld.py. The reason for that was running "ls" shell command
failed as it was evaluated in an environment with an empty path. This
has now been fixed with D77123, which ensures that all shell commands
inherit the host environment, so this patch should be safe to recommit.
The original commit message was:
A defensive check in ProcessLauncherWindows meant that we would never
attempt to launch a process with a completely empty environment -- the
host environment would be used instead. Instead, I make the function add
an extra null wchar_t at the end of an empty environment. The
documentation on this is a bit fuzzy, but it seems to be what is needed
to make windows accept these kinds of environments.
Reviewers: amccarth, friss
Differential Revision: https://reviews.llvm.org/D76835
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
This reverts commit because of test failures in TestHelloWorld.
It seems that this test (specifically running "ls" as a platform shell
command) depended on the implicit passing of the host environment.
The fix should be fairly simple (inherit the environment explicitly),
but it may take me a while to figure where exactly to do that. Revert
while I am figuring that out.
Summary:
A defensive check in ProcessLauncherWindows meant that we would never
attempt to launch a process with a completely empty environment -- the
host environment would be used instead. Instead, I make the function add
an extra null wchar_t at the end of an empty environment. The
documentation on this is a bit fuzzy, but it seems to be what is needed
to make windows accept these kinds of environments.
Reviewers: amccarth, friss
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76835
Commit 83c81c0a46 enabled Fix-Its for top-level
expressions which change the error message of this test here as Clang comes
up with a strange Fix-It for this expression. This patch just changes the
test to declare a void variable so that Clang doesn't see a way to
recover with a Fix-It and change the error message.
Summary:
Currently top-level expressions won't automatically get Fix-Its applied. The reason
for that is that we only set the `m_fixed_text` member if we have a wrapping
source code (I.e. `m_source_code` is not zero and is wrapping some expressions).
This patch just always sets `m_fixed_text` to get this working.
Reviewers: labath, jingham
Reviewed By: labath
Subscribers: JDevlieghere
Differential Revision: https://reviews.llvm.org/D77042
There an option: EvaluateExpressionOptions::SetResultIsInternal to indicate
whether the result number should be returned to the pool or not. It
got broken when the PersistentExpressionState was refactored.
This fixes the issue and provides a test of the behavior.
Differential Revision: https://reviews.llvm.org/D76532
The newly introduced tests for unsetting environment variables
is failing on Windows. Skip the test there to allow investigation.
It seems like setting inherit-env to false was never tested
before. Could it be that the Windows process launcher doesn't
honor this setting?
Summary:
The interactions between the environment settings (`target.env-vars`,
`target.inherit-env`) and the inferior life-cycle are non-obvious
today. For example, if `target.inherit-env` is set, the `target.env-vars`
setting will be augmented with the contents of the host environment
the first time the launch environment is queried (usually at
launch). After that point, toggling `target.inherit-env` will have no
effect as there's no tracking of what comes from the host and what is
a user setting.
This patch computes the environment every time it is queried rather
than updating the contents of the `target.env-vars` property. This
means that toggling the `target.inherit-env` property later will now
have the intended effect.
This patch also adds a `target.unset-env-vars` settings that one can
use to remove variables from the launch environment. Using this, you
can inherit all but a few of the host environment.
The way the launch environment is constructed is:
1/ if `target.inherit-env` is set, then read the host environment
into the launch environment.
2/ Remove for the environment the variables listed in
`target.unset-env`.
3/ Augment the launch environment with the contents of
`target.env-vars`. This overrides any common values with the host
environment.
The one functional difference here that could be seen as a regression
is that `target.env-vars` will not contain the inferior environment
after launch. The patch implements a better alternative in the
`target show-launch-environment` command which will return the
environment computed through the above rules.
Reviewers: labath, jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76470
Summary:
When no arguments or environment is provided to SBTarget::LaunchSimple,
make it use the values surrently set in the target properties. You can
get the current behavior back by passing an empty array instead.
It seems like using the target defaults is a much more intuitive
behavior for those APIs. It's unllikely that anyone passed NULL/None to
this API after having set properties in order to explicitely ignore them.
One direct application of this change is within the testsuite. We have
plenty of tests calling LaunchSimple and passing None as environment.
If you passed --inferior-env to dotest.py to, for example, set
(DY)LD_LIBRARY_PATH, it wouldn't be taken into account.
Reviewers: jingham, labath, #libc_abi!
Subscribers: libcxx-commits, lldb-commits
Tags: #lldb, #libc_abi
Differential Revision: https://reviews.llvm.org/D76045
Summary:
The TargetProperties constructor invokes a series of callbacks to
prime the properties from the default ones. The one callback in
charge of updating the inferior environment was commented out
because it crashed.
The reason for the crash is that TargetProperties is a parent class
of Target and the callbacks were invoked using a Target that was
not fully initialized. This patch moves the initial callback
invocations to a separate function that can be called at the end
the Target constructor, thus preventing the crash.
One existing test had to be modified, because the initialization of
the environment properties now take place at the time the target is
created, not at the first use of the environment (usually launch
time).
The added test checks that the LaunchInfo object returned by
the target has been primed with the values from the settings.
Reviewers: jingham, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76009
Summary:
LLDB keeps statistics of how many expression evaluations are 'successful' and 'failed'
which are updated after each expression evaluation (assuming statistics are enabled).
From what I understand the idea is that this could be used to define how well LLDB's
expression evaluator is working.
Currently all expressions are considered successful unless the user passes an explicit
positive element counting to the expression command (with the `-Z` flag) and then passes
an expression that successfully evaluates to a type that doesn't support element counting.
Expressions that fail to parse, execute or any other outcome are considered successful
at the moment which means we nearly always have a 100% expression evaluation
success rate.
This patch makes that expressions that fail to parse or execute to count as failed
expressions.
We can't know whether the expression failed because of an user error
of because LLDB couldn't correctly parse/compile it, but I would argue that this is
still an improvement. Assuming that the percentage of valid user expressions stays
mostly constant over time (which seems like a reasonable assumption), then this
way we can still see if we are doing relatively better/worse from release to release.
Reviewers: davide, aprantl, JDevlieghere
Reviewed By: aprantl
Subscribers: abidh
Differential Revision: https://reviews.llvm.org/D76280
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
There is still the bug that empty lines seem to skip any following expressions
and it makes it harder to commend between all the comments. Let's make this
a normal test instead which is just slightly more verbose but can be properly
formatted.
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
Since the `platform process` commamnd has more tests now, this commits
separates each of the `platform process` subcommand's test in its own directory.
Differential Revision: https://reviews.llvm.org/D74836
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
Around a third of our test sources have LLVM license headers. This patch removes those headers from all test
sources and also fixes any tests that depended on the length of the license header.
The reasons for this are:
* A few tests verify line numbers and will start failing if the number of lines in the LLVM license header changes. Once I landed my patch for valid SourceLocations in debug info we will probably have even more tests that verify line numbers.
* No other LLVM project is putting license headers in its test files to my knowledge.
* They make the test sources much more verbose than they have to be. Several tests have longer license headers than the actual test source.
For the record, the following tests had their line numbers changed to pass with the removal of the license header:
lldb-api :: functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
lldb-shell :: Reproducer/TestGDBRemoteRepro.test
lldb-shell :: Reproducer/TestMultipleTargets.test
lldb-shell :: Reproducer/TestReuseDirectory.test
lldb-shell :: ExecControl/StopHook/stop-hook-threads.test
lldb-shell :: ExecControl/StopHook/stop-hook.test
lldb-api :: lang/objc/exceptions/TestObjCExceptions.py
Reviewers: #lldb, espindola, JDevlieghere
Reviewed By: #lldb, JDevlieghere
Subscribers: emaste, aprantl, arphaman, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74839
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
All calls to operator new in this test fail for me with:
```
expression --show-types -- *(new foo(47))`
Error output:
error: Execution was interrupted, reason: internal c++ exception breakpoint(-6)..
The process has been returned to the state before expression evaluation.
```
As calling operator new isn't the idea of this test, this patch moves that
logic to the binary with some new_* utility functions and explicitly tests
this logic in the constructor test (where we can isolate the failures and
skip them on Linux).
Commit 82b47b2978 changes the way the stdlib.h
header is structured which seems to cause strange lookup failures in the modules
build. This updates a few failing tests so that they pass with the new
behavior of stdlib.h.
See the discussion in https://reviews.llvm.org/rG82b47b2978405f802a33b00d046e6f18ef6a47be
Summary:
The error message from the construct `assertTrue(a == b, "msg") ` are nearly always completely useless for actually debugging the issue.
This patch is just replacing this construct (and similar ones like `assertTrue(a != b, ...)` with the proper call to assertEqual or assertNotEquals.
This patch was mostly written by a shell script with some manual verification afterwards:
```
lang=python
import sys
def sanitize_line(line):
if line.strip().startswith("self.assertTrue(") and " == " in line:
line = line.replace("self.assertTrue(", "self.assertEquals(")
line = line.replace(" == ", ", ", 1)
if line.strip().startswith("self.assertTrue(") and " != " in line:
line = line.replace("self.assertTrue(", "self.assertNotEqual(")
line = line.replace(" != ", ", ", 1)
return line
for a in sys.argv[1:]:
with open(a, "r") as f:
lines = f.readlines()
with open(a, "w") as f:
for line in lines:
f.write(sanitize_line(line))
```
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74475
Summary: Moves lldbsuite tests to lldb/test/API.
This is a largely mechanical change, moved with the following steps:
```
rm lldb/test/API/testcases
mkdir -p lldb/test/API/{test_runner/test,tools/lldb-{server,vscode}}
mv lldb/packages/Python/lldbsuite/test/test_runner/test lldb/test/API/test_runner
for d in $(find lldb/packages/Python/lldbsuite/test/* -maxdepth 0 -type d | egrep -v "make|plugins|test_runner|tools"); do mv $d lldb/test/API; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-vscode -maxdepth 1 -mindepth 1 | grep -v ".py"); do mv $d lldb/test/API/tools/lldb-vscode; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-server -maxdepth 1 -mindepth 1 | egrep -v "gdbremote_testcase.py|lldbgdbserverutils.py|socket_packet_pump.py"); do mv $d lldb/test/API/tools/lldb-server; done
```
lldb/packages/Python/lldbsuite/__init__.py and lldb/test/API/lit.cfg.py were also updated with the new directory structure.
Reviewers: labath, JDevlieghere
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71151
Muhammad Omair Javaid