Dedicated completion for the command `thread plan discard` with a corresponding
test case.
Reviewed By: teemperor
Differential Revision: https://reviews.llvm.org/D83234
with how it is done for a lean binary
In particular this affects how target create --arch is handled — it
allowed us to override the deployment target (a useful feature for the
expression evaluator), but the fat binary case didn't.
rdar://problem/66024437
Differential Revision: https://reviews.llvm.org/D85049
(cherry picked from commit 470bdd3caaab0b6e0ffed4da304244be40b78668)
Add an option that allows the user to decide to not make the inferior is
responsible for its own TCC permissions. If you don't make the inferior
responsible, it inherits the permissions of its parent. The motivation
is the scenario of running the LLDB test suite from an external hard
drive. If the inferior is responsible, every test needs to be granted
access to the external volume. When the permissions are inherited,
approval needs to be granted only once.
Differential revision: https://reviews.llvm.org/D85237
On Hexagon, breakpoints need to be on the first instruction of a packet.
When the LLVM disassembler for Hexagon returned 32 bit instructions, we
needed code to find the start of the current packet. Now that the LLVM
disassembler for Hexagon returns packets instead of instructions, we always
have the first instruction of the packet. Remove the packet traversal code
because it can cause problems when the next packet has more than one
instruction.
Reviewed By: clayborg
Differential Revision: https://reviews.llvm.org/D84966
Let's just return a std::string to make this safe. formatv seemed overkill for formatting
the return values as they all just append an integer value to a constant string.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D84505
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
This cleanup patch unifies all methods called GetByteSize() in the
ValueObject hierarchy to return an optional, like the methods in
CompilerType do. This means fewer magic 0 values, which could fix bugs
down the road in languages where types can have a size of zero, such
as Swift and C (but not C++).
Differential Revision: https://reviews.llvm.org/D84285
This re-lands the patch with bogus :m_byte_size(0) initalizations removed.
This cleanup patch unifies all methods called GetByteSize() in the
ValueObject hierarchy to return an optional, like the methods in
CompilerType do. This means fewer magic 0 values, which could fix bugs
down the road in languages where types can have a size of zero, such
as Swift and C (but not C++).
Differential Revision: https://reviews.llvm.org/D84285
When a process is notified that modules got loaded, currently only
existing language runtimes are given a chance to deal with that. This
means that if the runtime for a given language wasn't needed before it
won't be informed of the module chance.
This is wrong because the module change might be what triggers the need
for a certain runtime. Instead, we should give the language runtime for
every supported language a chance to deal with the modified modules.
Differential revision: https://reviews.llvm.org/D84475
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
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
In synchronous mode, the process connect command and its aliases should
wait for the stop event before claiming the command is complete.
Currently, the stop event is always handled asynchronously by the
debugger.
The implementation takes the same approach as Process::ResumeSynchronous
which hijacks the event and handles it on the current thread. Similarly,
after this patch, the stop event is part of the command return object,
which is the property used by the test case.
Differential revision: https://reviews.llvm.org/D83728
Summary:
This patch aims to remove multiple copies of GetByteOrder() and ConvertRegisterKindToRegisterNumber used in various versions of RegisterContextPOSIX_*.
Both register implementations are move to RegisterContext class which is parent of RegisterContextPOSIX_* classes.
Built and tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabihf targets.
Reviewers: labath
Reviewed By: labath
Subscribers: wuzish, nemanjai, kristof.beyls, kbarton, atanasyan, lldb-commits
Differential Revision: https://reviews.llvm.org/D80104
Encountered the following situation: Let we started thread T1 and it hit
breakpoint on B1 location. We suspended T1 and continued the process.
Then we started thread T2 which hit for example the same location B1.
This time in a breakpoint callback we decided not to stop returning
false.
Expected result: process continues (as if T2 did not hit breakpoint) its
workflow with T1 still suspended. Actual result: process do stops (as if
T2 callback returned true).
Solution: We need invalidate StopInfo for threads that was previously
suspended just because something that is already inactive can not be the
reason of stop. Thread::GetPrivateStopInfo() may be appropriate place to
do it, because it gets called (through Thread::GetStopInfo()) every time
before process reports stop and user gets chance to change
m_resume_state again i.e if we see m_resume_state == eStateSuspended
it definitely means it was set during previous stop and it also means
this thread can not be stopped again (cos' it was frozen during
previous stop).
Differential revision: https://reviews.llvm.org/D80112
Color the error: and warning: part of the CommandReturnObject output,
similar to how an error is printed from the driver when colors are
enabled.
Differential revision: https://reviews.llvm.org/D81058
Summary:
The way that the support for the GNU dialect of tail call frames was
implemented in D80519 meant that the were reporting very bogus PC values
which pointed into the middle of an instruction: the -1 trick is
necessary for the address to resolve to the right function, but we
should still be reporting a more realistic PC value -- I say "realistic"
and not "real", because it's very debatable what should be the correct
PC value for frames like this.
This patch achieves that my moving the -1 from SymbolFileDWARF into the
stack frame computation code. The idea is that SymbolFileDWARF will
merely report whether it has provided an address of the instruction
after the tail call, or the address of the call instruction itself. The
StackFrameList machinery uses this information to set the "behaves like
frame zero" property of the artificial frames (the main thing this flag
does is it controls the -1 subtraction when looking up the function
address).
This required a moderate refactor of the CallEdge class, because it was
implicitly assuming that edges pointing after the call were real calls
and those pointing the the call insn were tail calls. The class now
carries this information explicitly -- it carries three mostly
independent pieces of information:
- an address of interest in the caller
- a bit saying whether this address points to the call insn or after it
- whether this is a tail call
Reviewers: vsk, dblaikie
Subscribers: aprantl, mgrang, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D81010
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>
Summary:
In our project we are using remote client-server LLDB configuration.
We want to parse as much debugging symbols as we can before debugger starts attachment to the remote process.
To do that we are passing the path of the local executable module to CreateTarget method at the client.
But, it seems that this method are not parsing the executable module symbols.
To fix this I added PreloadSymbols call for executable module to target creation method.
This patch also fixes a problem where the DynamicLoader would reset a
module when launching the target. We fix it by making sure
Platform::ResolveExecutable returns the module object obtained from the
remote platform.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D78654
The near-identical implementations of this function for posix-y
platforms were merged in r293910. PlatformWindows was left out of this
merge because at the time we did not have a suitable base class to sink
the code into. That is no longer true, so this commit finishes the job
by moving the code into RemoteAwarePlatform::ResolveExecutable.
Summary:
This fixes a bug where
frame var a[0]+5
returns the value a[0] without any warning because the current logic simply ignores everything after ']' as long as there is no '.', '-' or '[' in the rest of the string.
The fix simplifies the termination condition of the expression path parsing loop to check if have a non-empty remaining string to parse. Previously, the condition checked if a separator was found. That condition coincided with the remaining string-to-parse condition except for the buggy indexed case where non-empty string was left ("+5" in the example above), but the separator index was 'npos'.
Reviewed By: teemperor, labath
Differential Revision: https://reviews.llvm.org/D79404
Summary:
Languages can have different ways of formatting special characters.
E.g. when debugging C++ code a string might look like "\b", but when
debugging Swift code the same string would look like "\u{8}".
To make this work, plugins override GetStringPrinterEscapingHelper.
However, because there's a large amount of subtly divergent work done in
each override, we end up with large amounts of duplicated code. And all
the memory smashers fixed in one copy of the logic (see D73860) don't
get fixed in the others.
IMO the GetStringPrinterEscapingHelper is overly general and hard to
use. I propose deleting it and replacing it with an EscapeStyle enum,
which can be set as needed by each plugin.
A fix for some swift-lldb memory smashers falls out fairly naturally
from this deletion (https://github.com/apple/llvm-project/pull/1046). As
the swift logic becomes really tiny, I propose moving it upstream as
part of this change. I've added unit tests to cover it.
rdar://61419673
Reviewers: JDevlieghere, davide
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77843
This patch ensures we don't crash in GetSoftwareBreakpointTrapOpcode for
not-yet-supported architectures but rather continue with degraded
behavior.
I found the issue in the context of an invalid ArchSpec, which should be
handled further up the chain. In this patch I've also added an assert to
cover that, so we can still catch those issues.
Differential revision: https://reviews.llvm.org/D78588
Calling Disconnect while the read thread is running is racy because the
thread can also call Disconnect. This is a follow-up to b424b0bf, which
reorders other occurences of Disconnect/StopReadThread I can find, and also
adds an assertion to guard against new occurences being introduced.
Avoid a race between the Disconnect call in `Communication::ReadThread`
and the one in `Process::ShouldBroadcastEvent` by reordering the calls
to Disconnect and StopReadThread in `Process::ShouldBroadcastEvent`.
In D77295 Pavel suggested that that might explain the broken pipe I was
seeing. Indeed, changing the order resolved the issue.
Summary:
In D49685 sysroot behaviour was partially fixed. But files from local filesystem with same path still has priority over files from sysroot.
This patch fixes it by removing fallback to local filesystem from RemoteAwarePlatform::GetModuleSpec(). It is not actually required because higher level code do such fallback itself. See, for example, resolver in Platform::GetSharedModule().
Reviewers: labath, clayborg, EugeneBi
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77529
We would return `LLDB_INVALID_IMAGE_TOKEN` for the address rather than
the correct value of `LLDB_IMAGE_ADDRESS`. This would result in the
check for the return value to silently pass on x64 as the invalid
address and invalid token are of different sizes (`size_t` vs
`uintprr_t`). This corrects the return value to `LLDB_INVALID_ADDRESS`
and addresses the rest to reset the mapped address to the invalid value.
This was found by inspection when trying to implement module support for
Windows.
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:
A recent change in ThreadPlans introduced this little compilation error.
Seems to be related to the work around https://reviews.llvm.org/D76814.
Reviewers: clayborg, labath, jingham
Reviewed By: jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77450
that were not reported by the OS plugin. To facilitate this, move
adding/updating the ThreadPlans for a Thread to the ThreadPlanStackMap.
Also move dumping thread plans there as well.
Added some tests for "thread plan list" and "thread plan discard" since
I didn't seem to have written any originally.
Differential Revision: https://reviews.llvm.org/D76814
Reland with changes: the test modified in this change originally failed
on a Debian/x86_64 builder, and I suspect the cause was that lldb looked
up the line location for an artificial frame by subtracting 1 from the
frame's address. For artificial frames, the subtraction must not happen
because the address is already exact.
---
lldb currently guesses the address to use when creating an artificial
frame (i.e., a frame constructed by determining the sequence of (tail)
calls which must have happened).
Guessing the address creates problems -- use the actual address provided
by the DW_AT_call_pc attribute instead.
Depends on D76336.
rdar://60307600
Differential Revision: https://reviews.llvm.org/D76337
This reverts commit 6905394d15. The
changed test is failing on Debian/x86_64, possibly because lldb is
subtracting an offset from the DW_AT_call_pc address used for the
artificial frame:
http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/7171/steps/test/logs/stdio
/home/worker/lldb-x86_64-debian/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp:6:17: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] [artificial]
^
<stdin>:3:2: note: scanning from here
frame #1: 0x0000000000401127 a.out`func3() at main.cpp:13:4 [opt] [artificial]
lldb currently guesses the address to use when creating an artificial
frame (i.e., a frame constructed by determining the sequence of (tail)
calls which must have happened).
Guessing the address creates problems -- use the actual address provided
by the DW_AT_call_pc attribute instead.
Depends on D76336.
rdar://60307600
Differential Revision: https://reviews.llvm.org/D76337
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
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:
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
The fourth field in the property struct is the default unsigned or enum
value for all types, except for Array and Dictionary types. For those,
it is the element type. During the tablegen conversion, this was
incorrectly translated to DefaultValueUnsigned with a value
corresponding to the OptionValue: enum type. So for
OptionValue::eTypeString this became DefaultUnsignedValue<16>. This
patch extends the tablegen backend to understand ElementType to express
this as ElementType<"String">.
Differential revision: https://reviews.llvm.org/D76535
Summary:
This patch improves step over performance for the case when we are
stepping over a call with a next-branch-breakpoint (see
https://reviews.llvm.org/D58678), and we encounter a stop during the
call. Currently, this causes the thread plan to step-out //each frame//
until it reaches the step-over range. This is a regression introduced by
https://reviews.llvm.org/D58678 (which did improve other things!). Prior
to that change, the step-over plan would always step-out just once.
With this patch, if we find ourselves stopped in a deeper stack frame
and we already have a next branch breakpoint, we simply return from the
step-over plan's ShouldStop handler without pushing the step out plan.
In my experiments this improved the time of stepping over a call that
loads 12 dlls from 14s to 5s. This was in remote debugging scenario with
10ms RTT, the call in question was Vulkan initialization
(vkCreateInstance), which loads various driver dlls. Loading those dlls
must stop on the rendezvous breakpoint, causing the perf problem
described above.
Reviewers: clayborg, labath, jingham
Reviewed By: jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76216
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>
I believe the actual opcode does not matter because the AVR architecture
is a Harvard architecture that does not support writing to program
memory. Therefore, debuggers and emulators provide hardware breakpoints.
But for some reason, this opcode must be defined or else LLDB will crash
with an assertion error.
Differential Revision: https://reviews.llvm.org/D74255
Fix to code from https://reviews.llvm.org/D64993.
Field StackFrame::m_behaves_like_zeroth_frame was introduced in commit
[1], however that commit hasn't added a copying of the field to
UpdatePreviousFrameFromCurrentFrame, therefore the value wouldn't change
when updating frames to reflect the current situation.
The particular scenario, where this matters is following. Assume we have
function main that invokes function func1. We set breakpoint at
func1 entry and in main after the func1 call, and do not stop at
the main entry. Therefore, when debugger stops for the first time,
func1 is frame#0, while main is frame#1, thus
m_behaves_like_zeroth_frame is set to 0 for main frame. Execution is
resumed, and stops now in main, where it is now frame#0. However while
updating the frame object, m_behaves_like_zeroth_frame remains false.
This field plays an important role when calculating line information for
backtrace: for frame#0, PC is the current line, therefore line
information is retrieved for PC, however for all other frames this is
not the case - calculated PC is a return-PC, i.e. instruction after the
function call line, therefore for those frames LLDB needs to step back
by one instruction. Initial implementation did this strictly for frames
that have index != 0 (and index is updated properly in
UpdatePreviousFrameFromCurrentFrame), but m_behaves_like_zeroth_frame
added a capability for middle-of-stack frames to behave in a similar
manner. But because current code now doesn't check frame idx,
m_behaves_like_zeroth_frame must be set to true for frames with 0 index,
not only for frame that behave like one. In the described test case,
after stopping in main, LLDB would still consider frame#0 as
non-zeroth, and would subtract instruction from the PC, and would report
previous like as current line.
The error doesn't manifest itself in LLDB interpreter though - it can be
reproduced through LLDB-MI and when using SB API, but not when we
interpreter command "continue" is executed. Honestly, I didn't fully
understand why it works in interpreter, I did found that bug "fixes"
itself if I enable DEBUG_STACK_FRAMES in StackFrameList.cpp, because
that calls StackFrame::Dump and that calls
GetSymbolContext(eSymbolContextEverything), which fills the context of
frame on the first breakpoint, therefore it doesn't have to be
recalculated (improperly) on a second frame. However, on first
breakpoint symbol context is calculated for the "call" line, not the
next one, therefore it should be recalculated anyway on a second
breakpoint, and it is done correctly, even though
m_behaves_like_zeroth_frame is still incorrect, as long as
GetSymbolContext(eSymbolContextEverything) has been called.
[1] 31e6dbe1c6 Fix PC adjustment in StackFrame::GetSymbolContext
Differential Revision: https://reviews.llvm.org/D75975
Patch by Anton Kolesov <Anton.Kolesov@synopsys.com>
In addition to the commit rG352f16db87f583ec7f55f8028647b5fd8616111f,
this one fixes settings behavior on clearing - the setting should be
reverted to their default value, not an empty one.
Summary:
This is the only real unwinder, and things have been this way for quite
a long time. At this point, the class has accumulated so many features
it is unlikely that anyone will want to reimplement the whole thing.
The class is also fairly closely coupled (through UnwindPlans and
FuncUnwinders) with a lot of other lldb components that it is hard to
imagine a different unwinder implementation being substantially
different without reimplementing all of those.
The existing unwinding functionality is nonetheless fairly complex and
there is space for adding more structure to it, but I believe a more
worthwhile effort would be to take the existing UnwindLLDB class and try
to break it down and introduce extension/customization points, instead
of writing a brand new Unwind implementation.
Reviewers: jasonmolenda, JDevlieghere, xiaobai
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75848
Summary:
The class has two pairs of functions whose functionalities differ in
only how one specifies how much he wants to disasseble. One limits the
process by the size of the input memory region. The other based on the
total amount of instructions disassembled. They also differ in various
features (like error reporting) that were only added to one of the
versions.
There are various ways in which this could be addressed. This patch
does it by introducing a helper struct called "Limit", which is
effectively a pair specifying the value that you want to limit, and the
actual limit itself.
Reviewers: JDevlieghere
Subscribers: sdardis, jrtc27, atanasyan, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75730
Summary:
It isn't used anywhere (except on imaginary triples like
sparc-apple-ios) and it also violates plugin separation.
This patch deletes it and declares UnwindLLDB to be _the_ lldb unwinder.
Reviewers: jasonmolenda, JDevlieghere, xiaobai
Subscribers: jyknight, mgorny, krytarowski, fedor.sergeev, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75680
Some functions in this file only use the "target" component of an
execution context. Adjust the argument lists to reflect that.
This avoids some defensive null checks and simplifies most of the
callers.
This prevents calling Breakpoint::shared_from_this of an object that is not owned by any shared_ptr.
Differential Revision: https://reviews.llvm.org/D74557
Summary:
This gets rid of some nesting and of the raw char* variable that caused
the memory management bug we hit recently.
This commit also removes the fallback code which should trigger when
the StopInfo provides no stop description. All currently implemented
StopInfos have a `GetDescription()` method that shouldn't return an
empty description.
Reviewers: JDevlieghere, labath, mib
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74157
The lldb sanitizer bot is flagging a container-overflow error after we
introduced test TestWasm.py. MemoryCache::Read didn't behave correctly
in case of partial reads that can happen with object files whose size is
smaller that the cache size. It should return the actual number of bytes
read and not try to fill the buffer with random memory.
Module::GetMemoryObjectFile needs to be modified accordingly, to resize
its buffer to only the size that was read.
Differential Revision: https://reviews.llvm.org/D75200
Comparing those two `const char *` values relies on the assumption that both
strings were created by a ConstString. Let's check that assumption with an
assert as otherwise this code silently does nothing and that's not great.
The two classes are equivalent, except:
- the former uses a llvm::SmallVector (with a configurable size), while
the latter uses std::vector.
- the former has a typo in one of the functions name
This patch just leaves one class, using llvm::SmallVector, and defaults
the small size to zero. This is the same thing we did with the
RangeDataVector class in D56170.
Pass TargetSP to filters' CreateFromStructuredData, don't let them guess
whether target object is managed by a shared_ptr.
Make Breakpoint sure that m_target.shared_from_this() is safe by passing TargetSP
to all its static Create*** member-functions. This should be enough, since Breakpoint's
constructors are private/protected and never called directly (except by Target itself).
The error is: no matching function for call to 'transform(std::string&, std::__cxx11::basic_string<char>::iterator, <unresolved overloaded function type>)'
The fix: replace llvm::transform with an equally simple hand-rolled
loop.
Summary:
These definitions are used to "augment" information received from the remote
target with eh/debug frame and "generic" register numbers.
Besides being verbose, this information was also incomplete (new registers like
xmm16-31 were missing) and sometimes even downright wrong (ymm register
numbers).
Most of this information is available via llvm's MCRegisterInfo. This patch
creates a new class, MCBasedABI, which retrieves the eh and debug frame register
numbers this way. The tricky part here is that the llvm class uses all-caps
register names, whereas lldb register are lowercase, and sometimes called
slightly differently. Therefore this class introduces some hooks to allow a
subclass to customize the MC lookup. The subclass also needs to suply the
"generic" register numbers, as this is an lldb invention.
This patch ports the x86_64 ABI classes to use the new register info mechanism.
It also creates a new "ABIx86_64" class which can be used to house code common
to x86_64 both ABIs. Right now, this just consists of a single function, but
there are plenty of other things that could be moved here too.
Reviewers: JDevlieghere, jasonmolenda
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74244
SB API clients can describe the failure message in a more natural
way for their UI, this doesn't add information for them.
Differential Revision: https://reviews.llvm.org/D74585
<rdar://problem/49953304>
Summary:
This patch creates a new subclass of the ABI class in order to abstract away the
mechanism in which we "augment" register information. This enables alternate
augmentation strategies to be introduced.
All existing ABI classes have been modified to inherit from RegInfoBasedABI, but
they will be refactored in subsequent patches.
Reviewers: JDevlieghere, jasonmolenda
Subscribers: sdardis, nemanjai, kbarton, jrtc27, atanasyan, jsji, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74243
This reimplements commit 6b2979c123 and updates
the tests to reflect the addition of the alternate symbol attribute.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
StringRef will call strlen on the C string which is inefficient (as ConstString already
knows the string lenght and so does StringRef). This patch replaces all those calls
with GetStringRef() which doesn't recompute the length.
Summary:
The return address validation in D71372 will fail if the memory permissions can't be determined. Many embedded stubs either don't implement the qMemoryRegionInfo packet, or don't have memory permissions at all.
Remove the return from the if clause that calls GetLoadAddressPermissions, so this call failing doesn't cause the step out to abort. Instead, assume that the memory permission check doesn't apply to this type of target.
Reviewers: labath, jingham, clayborg, mossberg
Reviewed By: labath, jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72513
D73303 was failing on Fedora Linux and so it was disabled by Skip the
AssertFrameRecognizer test for Linux.
I find no easy way how to find out if it gets recognized as
`__assert_fail` or `__GI___assert_fail` as during `Process` ctor
libc.so.6 is not yet loaded by the debuggee.
DWARF symbol `__GI___assert_fail` overrides the ELF symbol `__assert_fail`.
While external debug info (=DWARF) gets disabled for testsuite (D55859)
that sure does not apply for real world usage.
Differential Revision: https://reviews.llvm.org/D74252
This reverts commit cf1046c716.
Reverted: https://reviews.llvm.org/D74252
It fixed testsuite but broke real world functionality where is not used:
settings set symbols.enable-external-lookup false
D73303 was failing on Fedora Linux and so it was disabled by Skip the
AssertFrameRecognizer test for Linux.
On Fedora 30 x86_64 I have:
$ readelf -Ws /lib64/libc.so.6 |grep '^Symbol\|.*assert_fail'
Symbol table '.dynsym' contains 2362 entries:
630: 0000000000030520 70 FUNC GLOBAL DEFAULT 14 __assert_fail@@GLIBC_2.2.5
Symbol table '.symtab' contains 22711 entries:
922: 000000000002275a 15 FUNC LOCAL DEFAULT 14 __assert_fail_base.cold
18044: 0000000000030520 70 FUNC LOCAL DEFAULT 14 __GI___assert_fail
20081: 00000000000303a0 370 FUNC LOCAL DEFAULT 14 __assert_fail_base
21766: 0000000000030520 70 FUNC GLOBAL DEFAULT 14 __assert_fail
The patch should never expect __GI___assert_fail:
.symtab can be present or not but that should not change that
__assert_fail always wins - it is always present from .dynsym and it can
never be overriden by __GI___assert_fail as __GI___assert_fail has only
local binding. Global binding is preferred since D63540.
External debug info symbols do not matter since D55859 (and DWARF should
never be embedded in system libc.so.6).
Differential Revision: https://reviews.llvm.org/D74252
One way to register a recognizer is to use RegularExpressionSP for the
module and symbol.
In order to match a symbol regardless of the module, the recognizer can
be registered with a nullptr for the module. However, this cause the
frame recognizer list command to crash because it calls
RegularExpression::GetText without checking if the shared pointer is valid.
This patch adds checks for the symbol and module RegularExpressionSP.
Differential Revision: https://reviews.llvm.org/D74212
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
When a thread stops, this checks depending on the platform if the top frame is
an abort stack frame. If so, it looks for an assert stack frame in the upper
frames and set it as the most relavant frame when found.
To do so, the StackFrameRecognizer class holds a "Most Relevant Frame" and a
"cooked" stop reason description. When the thread is about to stop, it checks
if the current frame is recognized, and if so, it fetches the recognized frame's
attributes and applies them.
rdar://58528686
Differential Revision: https://reviews.llvm.org/D73303
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Adrian Prantl