Perform all error handling in ReadCode()
Add :help text describing “< path”, add extra line before Commands
Differential Revision: https://reviews.llvm.org/D87640
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
This patch has no effect for C and C++. In more dynamic languages,
such as Objective-C and Swift GetByteSize() needs to call into the
language runtime, so it's important to pass one in where possible. My
primary motivation for this is some work I'm doing on the Swift
branch, however, it looks like we are also seeing warnings in
Objective-C that this may resolve. Everything in the SymbolFile
hierarchy still passes in nullptrs, because we don't have an execution
context in SymbolFile, since SymbolFile transcends processes.
Differential Revision: https://reviews.llvm.org/D84267
The `intrinsics_gen` target exists in the CMake exports since r309389
(see LLVMConfig.cmake.in), hence projects can depend on `intrinsics_gen`
even it they are built separately from LLVM.
Reviewed By: MaskRay, JDevlieghere
Differential Revision: https://reviews.llvm.org/D83454
This is an NFC cleanup for Clang, and a bugfix for the Swift
branch. In swift-lldb one target may have multiple scratch
TypeSystems, so it is important to pick the one that belongs to the
current frame, rather than the one for the current target.
<rdar://problem/65001402>
Summary:
When tabbing to complete LLDB commands in REPL, characters would at best be
missing but at worst cause the REPL to crash due to out of range string access.
This patch appends the command character to the completion results to fulfill
the assumption that all matches are prefixed by the request's cursor argument
prefix.
Bug report for the Swift REPL
https://bugs.swift.org/browse/SR-12867
Reviewers: teemperor
Reviewed By: teemperor
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D82835
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
The "type" argument to the function is mostly useless -- the only
interesting aspect of it is signedness. Pass signedness directly and
compute the value of bits and signedness fields -- that's exactly
what the single caller of this function does.
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
Previously, we were simply ignoring them and continuing the evaluation.
This behavior does not seem useful, because the resulting value will
most likely be completely bogus.
Summary:
The code changes are very straight-forward -- just handle both DW_AT_GNU
and DW_AT_call versions of all tags and attributes. There is just one
small gotcha: in the GNU version, DW_AT_low_pc was used both for the
"return pc" and the "call pc" values, depending on whether the tag was
describing a tail call, while the official scheme uses different
attributes for the two things.
Reviewers: vsk, dblaikie
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D80519
The llvm DWARFExpression dump is nearly identical, but better -- for
example it does print a spurious space after zero-argument expressions.
Some parts of our code (variable locations) have been already switched
to llvm-based expression dumping. This switches the remainder: unwind
plans and some unit tests.
This method has been commented as deprecated for a while. Remove
it and replace all uses with the equivalent getCalledOperand().
I also made a few cleanups in here. For example, to removes use
of getElementType on a pointer when we could just use getFunctionType
from the call.
Differential Revision: https://reviews.llvm.org/D78882
Originally committed as 416fa7720e
Reverted (due to buildbot failure - breaking lldb) in 7a45aeacf3.
I still can't seem to build lldb locally, but Pavel Labath has kindly
provided a potential fix to preserve the old behavior in lldb by
registering a simple recoverable error handler there that prints to the
desired stream in lldb, rather than stderr.
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
This patch fixes a crash that happens on the DWARF expression evaluator
when trying to access the top of the stack while it's empty.
rdar://60512489
Differential Revision: https://reviews.llvm.org/D77108
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch fixes a crash that happens on the DWARF expression evaluator
when trying to access the top of the stack while it's empty.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
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
Currently when an expression fails to parse and we have a FixIt, we keep
the failed UserExpression around while trying to parse the expression with
applied fixits. This means that we have this rather confusing control flow:
1. Original expression created and parsing attempted.
2. Expression with applied FixIts is created and parsing attempted.
3. Original expression is destroyed and parser deconstructed.
4. Expression with applied FixIts is destroyed and parser deconstructed.
This patch just deletes the original expression so that step 2 and 3 are
swapped and the whole process looks more like just sequentially parsing two
expressions (which is what we actually do here).
Doesn't fix anything just makes the code less fragile.
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
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.
UserExpression::GetJITModule was used to support an option in
UserExpression::Evaluate that let you hold onto the JIT Module used during
the expression evaluation. This was only actually used in one spot --
REPL::IOHandlerInputComplete. That method didn't actually take use the
JIT module it got back, so this feature was not used in practice.
This means that we can delete the support in UserExpression::Evaluate
and delete the UserExpression::GetJITModule method entirely.
Any REPL client should just move to CompletionRequest instead of relying on
the translation code from the old API, so let's remove that translation code.
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).
This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).
Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73258
and document the shortcomings of LLDB's partially defined DW_OP_piece
handling.
This would manifest as "DW_OP_piece for offset foo but top of stack is
of size bar".
rdar://problem/46262998
Differential Revision: https://reviews.llvm.org/D72880
By switching to Scalars that are backed by explicitly-sized APInts we
can avoid a bug that increases the buffer reserved for a small piece
to the next-largest host integer type.
This manifests as "DW_OP_piece for offset foo but top of stack is of size bar".
Differential Revision: https://reviews.llvm.org/D72879
Summary:
Whenever we cast an LLVM instruction to one of its subclasses, we do a double check if the RTTI
enum value actually allows us to cast the class. I don't see a way this can ever happen as even when
LLVM's RTTI system has some corrupt internal state (which we probably should not test in the first
place) we just reuse LLVM RTTI to do the second check.
This also means that if we ever make an actual programming error in this function (e.g., have a enum
value and then cast it to a different subclass), we just silently fall back to the JIT in our tests.
We also can't test this code in any reasonable way.
This removes the checks and uses `llvm::cast` instead which will raise a fatal error when casting fails.
Reviewers: labath, mib
Reviewed By: labath
Subscribers: abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72596
GetPersistentExpressionStateForLanguage() can return a nullptr if it
cannot construct a typesystem. This patch adds missing nullptr checks
at all uses.
Inspired by rdar://problem/58317195
Differential Revision: https://reviews.llvm.org/D72413
Summary:
Our code was expecting that a single (symbol) file contains only one
kind of location lists. This is not correct (on non-apple platforms, at
least) as a file can compile units with different dwarf versions.
This patch moves the deteremination of location list flavour down to the
compile unit level, fixing this problem. I have also tried to rougly
align the code with the llvm DWARFUnit. Fully matching the API is not
possible because of how lldb's DWARFExpression lives separately from the
rest of the DWARF code, but this is at least a step in the right
direction.
Reviewers: JDevlieghere, aprantl, clayborg
Subscribers: dblaikie, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71751
Eric Christopher