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
Currently the substrs parameter takes a list of strings that need to be
found but the ordering isn't checked. D73766 might change that so this
changes a several tests so that the order of the strings in the substrs
list is in the order in which they appear in the output.
Currently the substrs parameter takes a list of strings that need to be
found but the ordering isn't checked. D73766 might change that so this
changes a several tests so that the order of the strings in the substrs
list is in the order in which they appear in the output.
Currently the substrs parameter takes a list of strings that need to be
found but the ordering isn't checked. D73766 might change that so this
changes a several tests so that the order of the strings in the substrs
list is in the order in which they appear in the output.
Currently the substrs parameter takes a list of strings
that need to be found but the ordering isn't checked. D73766
might change that so this changes a several tests so that
the order of the strings in the substrs list is in the order
in which they appear in the output.
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>
BreakpointSites know they're backed by hardware based on whether the
"hardware index" is set. This does not appear the to be done for
arm/aarch64.
https://llvm.org/PR44659
Include whether or not a breakpoint is a hardware breakpoint in the
breakpoint location. This will show up in things like the breakpoint
list.
Differential revision: https://reviews.llvm.org/D73389
Recognize hardware breakpoints as breakpoints instead of just mach
exceptions. The mach exception is the same for watch and breakpoints, so
we have to try each to figure out which is which.
Differential revision: https://reviews.llvm.org/D73401
This error is caused by a combination of a couple of factors:
- the test accidentally creating a list with a single (empty) FileSpec
instead of an empty list
- lldb overzeleously converting empty strings into nullptrs
- asan overzeleously validating symlink(2) arguments (the real symlink
call would just fail with EFAULT)
I fix this by using FileSpec::GetPath instead of GetCString. This avoids
the nullptr and also avoids inserting the path into the global string
pool.
I also enhance the test case to test both empty paths and empty lists.
Summary:
The ValueObject code checks for a special `$$dereference$$` synthetic
child to allow formatter providers to implement a natural
dereferencing behavior in `frame variable` for objects like smart
pointers.
This support was broken when used directly throught the Python API and
not trhough `frame variable`. The reason is that
SBFrame.FindVariable() will return by default the synthetic variable
if it exists, while `frame variable` will not do this eagerly. The
code in `ValueObject::Dereference()` accounted for the latter but not
for the former. The fix is trivial. The test change includes
additional covergage for the already-working bahevior as it wasn't
covered by the testsuite before.
This commit also adds a short piece of documentatione explaining that
it is possible (even advisable) to provide this synthetic child
outstide of the range of the normal children.
Reviewers: jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73053
The way the IO handlers are currently managed by the debugger is wrong. The
implementation lacks proper synchronization between RunIOHandlerSync and
RunIOHandlers. The latter is meant to be run by the "main thread", while the
former is meant to be run synchronously, potentially from a different thread.
Imagine a scenario where RunIOHandlerSync is called from a different thread
than RunIOHandlers. Both functions manipulate the debugger's IOHandlerStack.
Although the push and pop operations are synchronized, the logic to activate,
deactivate and run IO handlers is not.
While investigating PR44352, I noticed some weird behavior in the Editline
implementation. One of its members (m_editor_status) was modified from another
thread. This happened because the main thread, while running RunIOHandlers
ended up execution the IOHandlerEditline created by the breakpoint callback
thread. Even worse, due to the lack of synchronization within the IO handler
implementation, both threads ended up executing the same IO handler.
Most of the time, the IO handlers don't need to run synchronously. The
exception is sourcing commands from external files, like the .lldbinit file.
I've added a (recursive) mutex to prevent another thread from messing with the
IO handlers wile another thread is running one synchronously. It has to be
recursive, because we might have to source another file when encountering a
command source in the original file.
Differential revision: https://reviews.llvm.org/D72748
Summary:
This code is handling debug info paths starting with /proc/self/cwd,
which is one of the mechanisms people use to obtain "relocatable" debug
info (the idea being that one starts the debugger with an appropriate
cwd and things "just work").
Instead of resolving the symlinks inside DWARFUnit, we can do the same
thing more elegantly by hooking into the existing Module path remapping
code. Since llvm::DWARFUnit does not support any similar functionality,
doing things this way is also a step towards unifying llvm and lldb
dwarf parsers.
Reviewers: JDevlieghere, aprantl, clayborg, jdoerfert
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71770
This test is just TestDataFormatterObjCNSData.py copied but without any changes
(and it therefore doesn't even test NSDate).
It's also failing as NSData has been changed by me in
4f244bba4f.
The test is currently failing on some systems with ASAN enabled due to:
```
==22898==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003da4 at pc 0x00010951c33d bp 0x7ffee6709e00 sp 0x7ffee67095c0
READ of size 5 at 0x603000003da4 thread T0
#0 0x10951c33c in wrap_memmove+0x16c (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x1833c)
#1 0x7fff4a327f57 in CFDataReplaceBytes+0x1ba (CoreFoundation:x86_64+0x13f57)
#2 0x7fff4a415a44 in __CFDataInit+0x2db (CoreFoundation:x86_64+0x101a44)
#3 0x1094f8490 in main main.m:424
#4 0x7fff77482084 in start+0x0 (libdyld.dylib:x86_64+0x17084)
0x603000003da4 is located 0 bytes to the right of 20-byte region [0x603000003d90,0x603000003da4)
allocated by thread T0 here:
#0 0x109547c02 in wrap_calloc+0xa2 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x43c02)
#1 0x7fff763ad3ef in class_createInstance+0x52 (libobjc.A.dylib:x86_64+0x73ef)
#2 0x7fff4c6b2d73 in NSAllocateObject+0x12 (Foundation:x86_64+0x1d73)
#3 0x7fff4c6b5e5f in -[_NSPlaceholderData initWithBytes:length:copy:deallocator:]+0x40 (Foundation:x86_64+0x4e5f)
#4 0x7fff4c6d4cf1 in -[NSData(NSData) initWithBytes:length:]+0x24 (Foundation:x86_64+0x23cf1)
#5 0x1094f8245 in main main.m:404
#6 0x7fff77482084 in start+0x0 (libdyld.dylib:x86_64+0x17084)
```
The reason is that we create a string "HELLO" but get the size wrong (it's 5 bytes instead
of 4). Later on we read the buffer and pretend it is 5 bytes long, causing an OOB read
which ASAN detects.
In general this test probably needs some cleanup as it produces on macOS 10.15 around
100 compiler warnings which isn't great, but let's first get the bot green.
This reverts D53469, which changed llvm's DWARF emission to emit
DW_AT_call_return_pc as a function-local offset. Such an encoding is not
compatible with post-link block re-ordering tools and isn't standards-
compliant.
In addition to reverting back to the original DW_AT_call_return_pc
encoding, teach lldb how to fix up DW_AT_call_return_pc when the address
comes from an object file pointed-to by a debug map. While doing this I
noticed that lldb's support for tail calls that cross a DSO/object file
boundary wasn't covered, so I added tests for that. This latter case
exercises the newly added return PC fixup.
The dsymutil changes in this patch were originally included in D49887:
the associated test should be sufficient to test DW_AT_call_return_pc
encoding purely on the llvm side.
Differential Revision: https://reviews.llvm.org/D72489
Summary:
This patch adds a new function to lldbtest: `expect_expr`. This function is supposed to replace the current approach
of calling `expect`/`runCmd` with `expr`, `p` etc.
`expect_expr` allows evaluating expressions and matching their value/summary/type/error message without
having to do any string matching that might allow unintended passes (e.g., `self.expect("expr 3+4", substrs=["7"])`
can unexpectedly pass for results like `(Class7) $0 = 7`, `(int) $7 = 22`, `(int) $0 = 77` and so on).
This only uses the function in a few places to test and demonstrate it. I'll migrate the tests in follow up commits.
Reviewers: JDevlieghere, shafik, labath
Reviewed By: labath
Subscribers: christof, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70314
Summary:
Motivation: When formatting an array of typedefed chars, we would like to display the array as a string.
The string formatter currently does not trigger because the formatter lookup does not resolve typedefs for array elements (this behavior is inconsistent with pointers, for those we do look through pointee typedefs). This patch tries to make the array formatter lookup somewhat consistent with the pointer formatter lookup.
Reviewers: teemperor, clayborg
Reviewed By: teemperor, clayborg
Subscribers: clayborg, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72133
qemu has a very small maximum packet size (4096) and it actually
only uses half of that buffer for some implementation reason,
so when lldb asks for the register target definitions, the x86_64
definition is larger than 4096/2 and we need to fetch it in two parts.
This patch and test is fixing a bug in
GDBRemoteCommunicationClient::ReadExtFeature when reading a target
file in multiple parts. lldb was assuming that it would always
get back the maximum packet size response (4096) instead of
using the actual size received and asking for the next group of
bytes.
We now have two tests in gdb_remote_client for unique features
of qemu - TestNestedRegDefinitions.py would test the ability
of lldb to follow multiple levels of xml includes; I opted to
create a separate TestRegDefinitionInParts.py test to test this
wrinkle in qemu's gdb remote serial protocol stub implementation.
Instead of combining both tests into a single test file.
<rdar://problem/49537922>
Now that we are building the python bindings on Windows once more, the
extended testsuite is running. Mark a few failing tests and skip a few
tests which hang. This should at least bring the bot back to green
without reverting the Python changes which are an improvement for the
build system and enable another ~35% of the test suite which was
previously disabled.
Summary:
As discussed on the mailing list [1] we have to make a decision for how to proceed with the modern-type-lookup.
This patch removes modern-type-lookup from LLDB. This just removes all the code behind the modern-type-lookup
setting but it does *not* remove any code from Clang (i.e., the ExternalASTMerger and the clang-import-test stay around
for now).
The motivation for this is that I don't think that the current approach of implementing modern-type-lookup
will work out. Especially creating a completely new lookup system behind some setting that is never turned on by anyone
and then one day make one big switch to the new system seems wrong. It doesn't fit into the way LLVM is developed and has
so far made the transition work much more complicated than it has to be.
A lot of the benefits that were supposed to come with the modern-type-lookup are related to having a better organization
in the way types move across LLDB and having less dependencies on unrelated LLDB code. By just looking at the current code (mostly
the ClangASTImporter) I think we can reach the same goals by just incrementally cleaning up, documenting, refactoring
and actually testing the existing code we have.
[1] http://lists.llvm.org/pipermail/lldb-dev/2019-December/015831.html
Reviewers: shafik, martong
Subscribers: rnkovacs, christof, arphaman, JDevlieghere, usaxena95, lldb-commits, friss
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71562
Summary:
Right now, NSException::GetSummary() has the following output:
"name: $exception_name - reason: $exception_reason"
It would be better to simplify the output by removing the name and only
showing the exception's reason. This way, annotations would look nicer in
the editor, and would be a shorter summary in the Variables Inspector.
Accessing the exception's name can still be done by expanding the
NSException object in the Variables Inspector.
rdar://54770115
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71311
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Summary:
A lot of our tests copied the setUp code from our TestSampleTest.py:
```
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
```
This code does nothing unless we actually do any setUp work in there, so let's remove all these method definitions.
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71454
Summary:
A lot of tests do this trick but the vast majority of them don't even call `print()`.
Most of this patch was generated by a script that just looks at all the files and deletes the line if there is no `print (` or `print(` anywhere else in the file.
I checked the remaining tests manually and deleted the import if we never call print (but instead do stuff like `expr print(...)` and similar false-positives).
I also corrected the additional empty lines after the import in the files that I manually edited.
Reviewers: JDevlieghere, labath, jfb
Reviewed By: labath
Subscribers: dexonsmith, wuzish, nemanjai, kbarton, christof, arphaman, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71452
of depending on it being set in the environment. Fred's change
from October assumed that SDKROOT was set in the environment
so that 'xcrun --show-sdk-path' would print the path. If it
was passed in as a Makefile variable, it wouldn't be set in
the environment and xcrun --show-sdk-path would always show the
macOS SDK path. When running the lldb testsuite against an ios
device via lit, this seems to be the case.
The cache in FormatCache uses only a type name as key. The hardcoded
formats, synthetic children, etc inspect an entire ValueObject to
determine their eligibility, which isn't modelled in the cache. This
leads to bugs such as the one in this patch (where two similarly named
types in different files have different hardcoded summary
providers). The problem is exaggerated in the Swift language plugin
due to the language's dynamic nature.
rdar://problem/57756763
Differential Revision: https://reviews.llvm.org/D71233
This test was accidentally passing on non-darwin OS because it was
explicitly setting the CFLAGS make variable. This meant that (in the
default config) it was building with absolutely no debug info, and so
setting a breakpoint on a stripped symbol failed, because there was
really no trace of it remaining. In other configurations, we were
generating the debug info (-gsplit-dwarf implies -g) and the test failed
because we did not treat the zeroed out debug info address specially.
The test was also xfailed in pretty much every non-standard
configuration.
This patch fixes the makefile to avoid messing with CFLAGS (use
CFLAGS_EXTRAS instead). This causes it to fail in all configurations
(except darwin), and so I replace the various decorators with a simple
os!=darwin check.
Summary:
One of the ways we try to make LLDB faster is by only creating the Clang declarations (and loading the associated types)
when we actually need them for something. For example an evaluated expression might need to load types to
type check and codegen the expression.
Currently this mechanism isn't really tested, so we currently have no way to know how many Clang nodes we load and
when we load them. In general there seems to be some confusion when and why certain Clang nodes are created.
As we are about to make some changes to the code which is creating Clang AST nodes we probably should have
a test that at least checks that the current behaviour doesn't change. It also serves as some kind of documentation
on the current behaviour.
The test in this patch is just evaluating some expressions and checks which Clang nodes are created due to this in the
module AST. The check happens by looking at the AST dump of the current module and then scanning it for the
declarations we are looking for.
I'm aware that there are things missing in this test (inheritance, template parameters, non-expression evaluation commands)
but I'll expand it in follow up patches.
Also this test found two potential bugs in LLDB which are documented near the respective asserts in the test:
1. LLDB seems to always load all types of local variables even when we don't reference them in the expression. We had patches
that tried to prevent this but it seems that didn't work as well as it should have (even though we don't complete these
types).
2. We always seem to complete the first field of any record we run into. This has the funny side effect that LLDB is faster when
all classes in a project have an arbitrary `char unused;` as their first member. We probably want to fix this.
Reviewers: shafik
Subscribers: abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71056
