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
Summary:
Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.
Bug: llvm.org/PR45310
Depends on D76804.
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: mgorny, jingham, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76805
Summary:
LLDB memory-maps large source files, and at the same time, caches
all source files in the Source Cache.
On Windows, memory-mapped source files are not writeable, causing
bad user experience in IDEs (such as errors when saving edited files).
IDEs should have the ability to disable the Source Cache at LLDB
startup, so that users can edit source files while debugging.
Bug: llvm.org/PR45310
Reviewers: labath, JDevlieghere, jingham
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76804
The synchronization logic in the previous had a subtle bug. Moving of
the "m_read_thread_did_exit = true" into the critical section made it
possible for some threads calling SynchronizeWithReadThread call to get
stuck. This could happen if there were already past the point where they
checked this variable. In that case, they would block on waiting for the
eBroadcastBitNoMorePendingInput event, which would never come as the
read thread was blocked on getting the synchronization mutex.
The new version moves that line out of the critical section and before
the sending of the eBroadcastBitNoMorePendingInput event, and also adds
some comments to explain why the things need to be in this sequence:
- m_read_thread_did_exit = true: prevents new threads for waiting on
events
- eBroadcastBitNoMorePendingInput: unblock any current thread waiting
for the event
- Disconnect(): close the connection. This is the only bit that needs to
be in the critical section, and this is to ensure that we don't close
the connection while the synchronizing thread is mucking with it.
Original commit message follows:
Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.
However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.
This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.
The fix is to hold the synchronization mutex while closing the
connection.
I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.
The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.
Reviewers: clayborg, JDevlieghere
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77295
Summary:
Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.
However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.
This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.
The fix is to hold the synchronization mutex while closing the
connection.
I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.
The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.
Reviewers: clayborg, JDevlieghere
Subscribers: mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77295
This is mostly useful for Swift support; it allows LLDB to substitute
a matching SDK it shipped with instead of the sysroot path that was
used at compile time.
The goal of this is to make the Xcode SDK something that behaves more
like the compiler's resource directory, as in that it ships with LLDB
rather than with the debugged program. This important primarily for
importing Swift and Clang modules in the expression evaluator, and
getting at the APINotes from the SDK in Swift.
For a cross-debugging scenario, this means you have to have an SDK for
your target installed alongside LLDB. In Xcode this will always be the
case.
rdar://problem/60640017
Differential Revision: https://reviews.llvm.org/D76471
Summary:
Detection of C strings does not work well for pointers. If the value object holding a (char*) pointer does not have an address (e.g., if it is a temp), the value is not considered a C string and its formatting is left to DumpDataExtractor rather than the special handling in ValueObject::DumpPrintableRepresentation. This leads to inconsistent outputs, e.g., in escaping non-ASCII characters. See the test for an example; the second test expectation is not met (without this patch). With this patch, the C string detection only insists that the pointer value is valid. The patch makes the code consistent with how the pointer is obtained in ValueObject::ReadPointedString.
Reviewers: teemperor
Reviewed By: teemperor
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76650
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 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:
Otherwise this code won't run on the Release+Asserts builds we have on the CI.
Fixes rdar://problem/59867885 (partly)
Reviewers: aprantl
Reviewed By: aprantl
Subscribers: JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75493
The static Disassembler can be thought of as shorthands for three
operations:
- fetch an appropriate disassembler instance (FindPluginForTarget)
- ask it to dissassemble some bytes (ParseInstructions)
- ask it to dump the disassembled instructions (PrintInstructions)
The only thing that's standing in the way of this interpretation is that
the Disassemble function also does some address resolution before
calling ParseInstructions. This patch moves this functionality into
ParseInstructions so that it is available to users who call
ParseInstructions directly.
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.
the previously static member function took a Disassembler* argument
anyway. This renames the argument to "this". The function also always
succeeds (returns true), so I change the return type to void.
by "inlining" them into their single caller (CommandObjectDisassemble).
The functions mainly consist of long argument lists and defensive
checks. These become unnecessary after inlining, so the end result is
less code. Additionally, this makes the implementation of
CommandObjectDisassemble more uniform (first figure out what you're
going to disassemble, then actually do it), which enables further
cleanups.
Instead of a ExecutionContext*. All it needs is the target so it can
read the memory.
This removes some defensive checks from the function. I've added
equivalent checks to the callers in cases where a non-null target
pointer was not guaranteed to be available.
Summary:
If a command from a sourced file produces asynchronous output, this
output often does not make its way to the user. This happens because the
asynchronous output machinery relies on the iohandler stack to ensure
the output does not interfere with the things the iohandler is doing.
However, if this happens near the end of the command stream then by the
time the asynchronous output is produced we may already have already
started tearing down the sourcing session. Specifically, we may already
pop the relevant iohandler, leaving the stack empty.
This patch makes sure this kind of output gets printed by adding a
fallback to IOHandlerStack::PrintAsync to print the output directly if
the stack is empty. This is safe because if we have no iohandlers then
there is nothing to synchronize.
Reviewers: JDevlieghere, clayborg
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75454
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
Highlight the color marker similar to what we do for the column marker.
The default color matches the color of the current PC marker (->) in the
default disassembly format.
Differential revision: https://reviews.llvm.org/D75070
The only thing needed was to account for the offset from the
debug_cu_index section when searching for the location list.
This patch also fixes a bug in the Module::ParseAllDebugSymbols
function, which meant that we would only parse the variables of the
first compile unit in the module. This function is only used from
lldb-test, so this does not fix any real issue, besides preventing me
from writing a test for this patch.
The PluginManager contains a lot of duplicate code. I already removed a
bunch of it by introducing the templated PluginInstance class, and this
is the next step. The PluginInstances class combines the mutex and the
vector and implements the common operations.
To accommodate plugin instances with additional members it is possible
to access the underlying vector and mutex. The methods to query these
fields make use of that.
Differential revision: https://reviews.llvm.org/D74816
The plugin manager had dedicated Get*PluginCreateCallbackForPluginName
methods for each type of plugin, and only a small subset of those were
used. This removes the dead duplicated code.
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).
Summary:
All of our lookup APIs either use `CompilerDeclContext &` or `CompilerDeclContext *` semi-randomly it seems.
This leads to us constantly converting between those two types (and doing nullptr checks when going from
pointer to reference). It also leads to the confusing situation where we have two possible ways to express
that we don't have a CompilerDeclContex: either a nullptr or an invalid CompilerDeclContext (aka a default
constructed CompilerDeclContext).
This moves all APIs to use references and gets rid of all the nullptr checks and conversions.
Reviewers: labath, mib, shafik
Reviewed By: labath, shafik
Subscribers: shafik, arphaman, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74607
Beside these two functions just being wrappers around GetTypeName they are also
just a leftover from migrating the CompilerType interface to ConstString.
Summary:
The only use of this class was to implement the SharedCluster of ValueObjects.
However, the same functionality can be implemented using a regular
std::shared_ptr, and its little-known "sub-object pointer" feature, where the
pointer can point to one thing, but actually delete something else when it goes
out of scope.
This patch reimplements SharedCluster using this feature --
SharedClusterPointer::GetObject now returns a std::shared_pointer which points
to the ValueObject, but actually owns the whole cluster. The only change I
needed to make here is that now the SharedCluster object needs to be created
before the root ValueObject. This means that all private ValueObject
constructors get a ClusterManager argument, and their static Create functions do
the create-a-manager-and-pass-it-to-value-object dance.
Reviewers: teemperor, JDevlieghere, jingham
Subscribers: mgorny, jfb, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74153
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.
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>
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>
On Fedora 30 x86_64 with
cmake ../llvm-monorepo/llvm/ -DCMAKE_BUILD_TYPE=Debug -DLLVM_USE_LINKER=gold -DLLVM_ENABLE_PROJECTS="lldb;clang;lld" -DLLVM_USE_SPLIT_DWARF=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ENABLE_ASSERTIONS=ON
It does not affect Release builds.
getting:
lldb/source/Expression/IRInterpreter.cpp:1471: error: undefined reference to 'lldb_private::ThreadPlanCallFunctionUsingABI::ThreadPlanCallFunctionUsingABI(lldb_private::Thread&, lldb_private::Address const&, llvm::Type&, llvm::Type&, llvm::ArrayRef<lldb_private::ABI::CallArgument>, lldb_private::EvaluateExpressionOptions const&)'
lldb/source/Expression/LLVMUserExpression.cpp:148: error: undefined reference to 'lldb_private::ThreadPlanCallUserExpression::ThreadPlanCallUserExpression(lldb_private::Thread&, lldb_private::Address&, llvm::ArrayRef<unsigned long>, lldb_private::EvaluateExpressionOptions const&, std::shared_ptr<lldb_private::UserExpression>&)'
Pavel Labath has suggest LINK_INTERFACE_MULTIPLICITY could be further
increased.
Differential Revision: https://reviews.llvm.org/D73847
I previously removed the code in ValueObject::GetExpressionPath that
took advantage of the parameter `qualify_cxx_base_classes`. As a result,
this is now unused and can be removed.
Value::GetValueByteSize() reports the size of a Value as the size of its
underlying CompilerType. However, a host buffer that backs a Value may
be smaller than GetValueByteSize().
This situation arises when the host is only able to partially evaluate a
Value, e.g. because the expression contains DW_OP_piece.
The cleanest fix I've found to this problem is Greg's suggestion, which
is to resize the Value if (after evaluating an expression) it's found to
be too small. I've tried several alternatives which all (in one way or
the other) tried to teach the Value/ValueObjectChild system not to read
past the end of a host buffer, but this was flaky and impractical as it
isn't easy to figure out the host buffer's size (Value::GetScalar() can
point to somewhere /inside/ a host buffer, but you need to walk up the
ValueObject hierarchy to try and find its size).
This fixes an ASan error in lldb seen when debugging a clang binary.
I've added a regression test in test/functionalities/optimized_code. The
point of that test is not specifically to check that DW_OP_piece is
handled a particular way, but rather to check that lldb doesn't crash on
an input that it used to crash on.
Testing: check-lldb, and running the added tests using a sanitized lldb
--
Thanks to Jim for pointing out that an earlier version of this patch,
which simply changed the definition of Value::GetValueByteSize(), would
interact poorly with the ValueObject machinery.
Thanks also to Pavel who suggested a neat way to test this change
(which, incidentally, caught another ASan issue still present in the
original version of this patch).
rdar://58665925
Differential Revision: https://reviews.llvm.org/D73148
Emre Kultursay