lldb had three preprocessor defines for logging,
LLDB_LOG - formatv style argument
LLDB_LOGF - printf style argument
LLDB_LOGV - formatv style argument, only when verbose enabled
If you weren't looking at Log.h and the definition of these three, and
wanted to log something with formatv, it was easy to use LLDB_LOGV by
accident. We just had a situation where an important log statement
wasn't logging and it turned out to be this. This is fragile if you
aren't looking at the header directly, so I'd like to make this more
explicit. My proposal:
LLDB_LOG - formatv style argument
LLDB_LOG_VERBOSE - formatv style argument, only when verbose enabled
LLDB_LOGF - printf style argument
LLDB_LOGF_VERBOSE - printf style argument, only when verbose enabled
The new fouth one is to remove several places where we do `if (log &&
log->GetVerbose()) LLDB_LOGF (...)` in the sources today, and make both
styles consistent.
This PR implements that change, mechanically changing all LLDB_LOGV's to
LLDB_LOG_VERBOSE.
It also updates many of the `if (log && log->GetVerbose()) LLDB_LOGF`'s.
Some uses of this conditional expression do extra calculations in
addition to logging, and so those were left as-is so we're not doing
throwaway work when running without verbose logging.
There were many instances throughout lldb where callers are still doing
`if (log) LLDB_LOG*(...)`, a remnant of when all calls were to the `Log`
object's `Printf()` method, and you had to check if your local Log*
pointer was non-nullptr before calling the method. I removed those,
again keeping ones where work for logging is done in the block of code.
The code changes are all mechanical and uninteresting, but the question
of whether this naming change is widely agreed on is maybe worth
discussing.
Jonas recently added a trampoline handling strategy for simple language
thunks that does: "step through language thunks stepping in one level
deep and stopping if you hit user code". That was actually pulled over
from the swift implementation. However, this strategy and the strategy
we have to "step out past language thunks" when stepping out come into
conflict if the thunk you are stepping through calls some other function
before dispatching to the intended method. When you step out of the
called function back into the thunk, should you keep stepping out past
the thunk or not?
In most cases, you want to step out past the thunk, but in this
particular case you don't.
This patch adds a way to inform the thread plan (or really it's
ShouldStopHere behavior) of which behavior it should have, and passes
the don't step through thunks to the step through plan it uses to step
through thunks.
I didn't add a test for this because I couldn't find a C++ thunk that
calls another function before getting to the target function. I asked
the clang folks here if they could think of a case where clang would do
this, and they couldn't. If anyone can think of such a construct, it
will be easy to write the step through test for it...
This does happen in swift, however, so when I cherry-pick this to the
swift fork I'll test it there.
Lots of code around LLDB was directly accessing the target's section
load list. This NFC patch makes the section load list private so the
Target class can access it, but everyone else now uses accessor
functions. This allows us to control the resolving of addresses and will
allow for functionality in LLDB which can lazily resolve addresses in
JIT plug-ins with a future patch.
Many calls to Function::GetAddressRange() were not interested in the
range itself. Instead they wanted to find the address of the function
(its entry point) or the base address for relocation of function-scoped
entities (technically, the two don't need to be the same, but there's
isn't good reason for them not to be). This PR creates a separate
function for retrieving this, and changes the existing
(non-controversial) uses to call that instead.
The computation of 'Thread::IsVirtualStep" was wrong - it called being
at the bottom of a virtual call stack a "virtual step" but that is
actually when you've gotten to concrete code and need to step for real.
I also added a test for this.
This fixes the two test suite failures that I missed in the PR:
https://github.com/llvm/llvm-project/pull/112939
One was a poorly written test case - it assumed that on connect to a
gdb-remote with a running process, lldb MUST have fetched all the frame
0 registers. In fact, there's no need for it to do so (as the CallSite
patch showed...) and if we don't need to we shouldn't. So I fixed the
test to only expect a `g` packet AFTER calling read_registers.
The other was a place where some code had used 0 when it meant
LLDB_INVALID_LINE_NUMBER, which I had fixed but missed one place where
it was still compared to 0.
…ne stepping (#112939)"
This was breaking some gdb-remote packet counting tests on the bots. I
can't see how this patch could cause that breakage, but I'm reverting to
figure that out.
This reverts commit f14743794587db102c6d1b20f9c87a1ac20decfd.
Previously lldb didn't support setting breakpoints on call site
locations. This patch adds that ability.
It would be very slow if we did this by searching all the debug
information for every inlined subroutine record looking for a call-site
match, so I added one restriction to the call-site support. This change
will find all call sites for functions that also supply at least one
line to the regular line table. That way we can use the fact that the
line table search will move the location to that subsequent line (but
only within the same function). When we find an actually moved source
line match, we can search in the function that contained that line table
entry for the call-site, and set the breakpoint location back to that.
When I started writing tests for this new ability, it quickly became
obvious that our support for virtual inline stepping was pretty buggy.
We didn't print the right file & line number for the breakpoint, and we
didn't set the position in the "virtual inlined stack" correctly when we
hit the breakpoint. We also didn't step through the inlined frames
correctly. There was code to try to detect the right inlined stack
position, but it had been refactored a while back with the comment that
it was super confusing and the refactor was supposed to make it clearer,
but the refactor didn't work either.
That code was made much clearer by abstracting the job of "handling the
stack readjustment" to the various StopInfo's. Previously, there was a
big (and buggy) switch over stop info's. Moving the responsibility to
the stop info made this code much easier to reason about.
We also had no tests for virtual inlined stepping (our inlined stepping
test was actually written specifically to avoid the formation of a
virtual inlined stack... So I also added tests for that along with the
tests for setting the call-site breakpoints.
This PR introduces a new `ThreadPlanSingleThreadTimeout` that will be
used to address potential deadlock during single-thread stepping.
While debugging a target with a non-trivial number of threads (around
5000 threads in one example target), we noticed that a simple step over
can take as long as 10 seconds. Enabling single-thread stepping mode
significantly reduces the stepping time to around 3 seconds. However,
this can introduce deadlock if we try to step over a method that depends
on other threads to release a lock.
To address this issue, we introduce a new
`ThreadPlanSingleThreadTimeout` that can be controlled by the
`target.process.thread.single-thread-plan-timeout` setting during
single-thread stepping mode. The concept involves counting the elapsed
time since the last internal stop to detect overall stepping progress.
Once a timeout occurs, we assume the target is not making progress due
to a potential deadlock, as mentioned above. We then send a new async
interrupt, resume all threads, and `ThreadPlanSingleThreadTimeout`
completes its task.
To support this design, the major changes made in this PR are:
1. `ThreadPlanSingleThreadTimeout` is popped during every internal stop
and reset (re-pushed) to the top of the stack (as a leaf node) during
resume. This is achieved by always returning `true` from
`ThreadPlanSingleThreadTimeout::DoPlanExplainsStop()` and
`ThreadPlanSingleThreadTimeout::MischiefManaged()`.
2. A new thread-specific async interrupt stop is introduced, which can
be detected/consumed by `ThreadPlanSingleThreadTimeout`.
3. The clearing of branch breakpoints in the range thread plan has been
moved from `DoPlanExplainsStop()` to `ShouldStop()`, as it is not
guaranteed that it will be called.
The detailed design is discussed in the RFC below:
[https://discourse.llvm.org/t/improve-single-thread-stepping/74599](https://discourse.llvm.org/t/improve-single-thread-stepping/74599)
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
Ensure step-avoid-regexp logs are emitted in the case where the regex has no
capture groups.
Without this change, the log is printed only if the regex has at least one
capture group.
Another change is to the log message: the first capture group has been removed
from the message. There could be zero capture groups, and there could be two or
more capture groups.
Differential Revision: https://reviews.llvm.org/D119298
Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
infrastructure). This worked because Log.h included Logging.h, even
though it should.
After the recent refactor, it became impossible the two files include
each other in this direction (the opposite inclusion is needed), so this
patch removes the workaround that was put in place and cleans up all
files to include the right thing. It also renames the file to LLDBLog to
better reflect its purpose.
A long time ago LLDB wanted to start using StringRef instead of
C-Strings/ConstString but was blocked by the fact that the StringRef constructor
that takes a C-string was asserting that the C-string isn't a nullptr. To
workaround this, D24697 introduced a special function called `withNullAsEmpty`
and that's what LLDB (and only LLDB) started to use to build StringRefs from
C-strings.
A bit later it seems that `withNullAsEmpty` was declared too awkward to use and
instead the assert in the StringRef constructor got removed (see D24904). The
rest of LLDB was then converted to StringRef by just calling the now perfectly
usable implicit constructor.
However, all the calls to `withNullAsEmpty` just remained and are now just
strange artefacts in the code base that just look out of place. It's also
curiously a LLDB-exclusive function and no other project ever called it since
it's introduction half a decade ago.
This patch removes all uses of `withNullAsEmpty`. The follow up will be to
remove the function from StringRef.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D102597
`QueueThreadPlanForStepInRange` accepts a `step_into_target`, but the constructor for
`ThreadPlanStepInRange` does not. Instead, a caller would optionally call
`SetStepInTarget()` in a separate statement.
This change adds `step_into_target` as a constructor argument. This simplifies
construction of `ThreadPlanSP`, by avoiding a subsequent downcast and conditional
assignment. This constructor is already used in downstream repos.
Differential Revision: https://reviews.llvm.org/D96539
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
Summary:
Yet another step on the long road towards getting rid of lldb's Stream class.
We probably should just make this some kind of member of Address/AddressRange, but it seems quite often we just push
in random integers in there and this is just about getting rid of Stream and not improving arbitrary APIs.
I had to rename another `DumpAddress` function in FormatEntity that is dumping the content of an address to make Clang happy.
Reviewers: labath
Reviewed By: labath
Subscribers: JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71052
Summary:
The FileSpec class is often used as a sort of a pattern -- one specifies
a bare file name to search, and we check if in matches the full file
name of an existing module (for example).
These comparisons used FileSpec::Equal, which had some support for it
(via the full=false argument), but it was not a good fit for this job.
For one, it did a symmetric comparison, which makes sense for a function
called "equal", but not for typical searches (when searching for
"/foo/bar.so", we don't want to find a module whose name is just
"bar.so"). This resulted in patterns like:
if (FileSpec::Equal(pattern, file, pattern.GetDirectory()))
which would request a "full" match only if the pattern really contained
a directory. This worked, but the intended behavior was very unobvious.
On top of that, a lot of the code wanted to handle the case of an
"empty" pattern, and treat it as matching everything. This resulted in
conditions like:
if (pattern && !FileSpec::Equal(pattern, file, pattern.GetDirectory())
which are nearly impossible to decipher.
This patch introduces a FileSpec::Match function, which does exactly
what most of FileSpec::Equal callers want, an asymmetric match between a
"pattern" FileSpec and a an actual FileSpec. Empty paterns match
everything, filename-only patterns match only the filename component.
I've tried to update all callers of FileSpec::Equal to use a simpler
interface. Those that hardcoded full=true have been changed to use
operator==. Those passing full=pattern.GetDirectory() have been changed
to use FileSpec::Match.
There was also a handful of places which hardcoded full=false. I've
changed these to use FileSpec::Match too. This is a slight change in
semantics, but it does not look like that was ever intended, and it was
more likely a result of a misunderstanding of the "proper" way to use
FileSpec::Equal.
[In an ideal world a "FileSpec" and a "FileSpec pattern" would be two
different types, but given how widespread FileSpec is, it is unlikely
we'll get there in one go. This at least provides a good starting point
by centralizing all matching behavior.]
Reviewers: teemperor, JDevlieghere, jdoerfert
Subscribers: emaste, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70851
I find as a good cleanup to drop the Compile method. As I do not find TIMTOWTDI
as an advantage and there is already constructor parameter to compile the
regex.
Differential Revision: https://reviews.llvm.org/D66392
llvm-svn: 369352
Originally I wanted to remove the RegularExpression class in Utility and
replace it with llvm::Regex. However, during that transition I noticed
that there are several places where need the regular expression string.
So instead I propose to keep the RegularExpression class and make it a
thin wrapper around llvm::Regex.
This patch also removes the workaround for empty regular expressions.
The result is that we are now (more or less) POSIX conformant.
Differential revision: https://reviews.llvm.org/D66174
llvm-svn: 369153
This patch replaces explicit calls to log::Printf with the new LLDB_LOGF
macro. The macro is similar to LLDB_LOG but supports printf-style format
strings, instead of formatv-style format strings.
So instead of writing:
if (log)
log->Printf("%s\n", str);
You'd write:
LLDB_LOG(log, "%s\n", str);
This change was done mechanically with the command below. I replaced the
spurious if-checks with vim, since I know how to do multi-line
replacements with it.
find . -type f -name '*.cpp' -exec \
sed -i '' -E 's/log->Printf\(/LLDB_LOGF\(log, /g' "{}" +
Differential revision: https://reviews.llvm.org/D65128
llvm-svn: 366936
A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
The `ap` suffix is a remnant of lldb's former use of auto pointers,
before they got deprecated. Although all their uses were replaced by
unique pointers, some variables still carried the suffix.
In r353795 I removed another auto_ptr remnant, namely redundant calls to
::get for unique_pointers. Jim justly noted that this is a good
opportunity to clean up the variable names as well.
I went over all the changes to ensure my find-and-replace didn't have
any undesired side-effects. I hope I didn't miss any, but if you end up
at this commit doing a git blame on a weirdly named variable, please
know that the change was unintentional.
llvm-svn: 353912
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
When debugging read-only memory we cannot use software breakpoint. We
already have support for hardware breakpoints and users can specify them
with `-H`. However, there's no option to force LLDB to use hardware
breakpoints internally, for example while stepping.
This patch adds a setting target.require-hardware-breakpoint that forces
LLDB to always use hardware breakpoints. Because hardware breakpoints
are a limited resource and can fail to resolve, this patch also extends
error handling in thread plans, where breakpoints are used for stepping.
Differential revision: https://reviews.llvm.org/D54221
llvm-svn: 346920
This patch removes the comments grouping header includes. They were
added after running IWYU over the LLDB codebase. However they add little
value, are often outdates and burdensome to maintain.
llvm-svn: 346626
This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.
FYI, the script I used was:
import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue
if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)
continue
if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""
out.write(line)
os.rename(tmp, sys.argv[1])
Differential Revision: https://reviews.llvm.org/D46144
llvm-svn: 331197
This reverts commit r327318. It breaks the Xcode and CMake Darwin
builders:
clang: error: no such file or directory:
'.../source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp'
clang: error: no input files
More details are in https://reviews.llvm.org/D42582.
llvm-svn: 327327
All references to Host and Core have been removed, so this
class can now safely be lowered into Utility.
Differential Revision: https://reviews.llvm.org/D30559
llvm-svn: 296909
This moves the following classes from Core -> Utility.
ConstString
Error
RegularExpression
Stream
StreamString
The goal here is to get lldbUtility into a state where it has
no dependendencies except on itself and LLVM, so it can be the
starting point at which to start untangling LLDB's dependencies.
These are all low level and very widely used classes, and
previously lldbUtility had dependencies up to lldbCore in order
to use these classes. So moving then down to lldbUtility makes
sense from both the short term and long term perspective in
solving this problem.
Differential Revision: https://reviews.llvm.org/D29427
llvm-svn: 293941
This updates getters and setters to use StringRef instead of
const char *. I tested the build on Linux, Windows, and OSX
and saw no build or test failures. I cannot test any BSD
or Android variants, however I expect the required changes
to be minimal or non-existant.
llvm-svn: 282079
*** to conform to clang-format’s LLVM style. This kind of mass change has
*** two obvious implications:
Firstly, merging this particular commit into a downstream fork may be a huge
effort. Alternatively, it may be worth merging all changes up to this commit,
performing the same reformatting operation locally, and then discarding the
merge for this particular commit. The commands used to accomplish this
reformatting were as follows (with current working directory as the root of
the repository):
find . \( -iname "*.c" -or -iname "*.cpp" -or -iname "*.h" -or -iname "*.mm" \) -exec clang-format -i {} +
find . -iname "*.py" -exec autopep8 --in-place --aggressive --aggressive {} + ;
The version of clang-format used was 3.9.0, and autopep8 was 1.2.4.
Secondly, “blame” style tools will generally point to this commit instead of
a meaningful prior commit. There are alternatives available that will attempt
to look through this change and find the appropriate prior commit. YMMV.
llvm-svn: 280751
If the function is a template then the return type is part of the
function name. This CL fixes the parsing of these function names in
the case when the return type contains ':'.
The name of free functions in C++ don't have context part. Fix the
logic geting the function name without arguments out from a full
function name to handle this case.
Change the handling of step-in-avoid-regexp to match the value against
the function name without it's arguments and return value. This is
required because the default regex ("^std::") would match any template
function returning an std object.
Fifferential revision: http://reviews.llvm.org/D11461
llvm-svn: 243099
but that wasn't added to the list of reasons they don't explain. That
would mean we keep stepping after hitting the AsanDie breakpoint rather
than stopping when the Asan event occurred.
<rdar://problem/21925479>
llvm-svn: 243035
So that we don't have to update every single #include in the entire
codebase to #include this new header (which used to get included by
lldb-private-log.h, we automatically #include "Logging.h" from
within "Log.h".
llvm-svn: 232653
the user level. It adds the ability to invent new stepping modes implemented by python classes,
and to view the current thread plan stack and to some extent alter it.
I haven't gotten to documentation or tests yet. But this should not cause any behavior changes
if you don't use it, so its safe to check it in now and work on it incrementally.
llvm-svn: 218642
the same parent frame, but different current frame - e.g. when
you step past a tail call exit from a function. Apply the same
"avoid-no-debug" rules to this case as for a "step-in".
<rdar://problem/16189225>
llvm-svn: 214946
for customizing "step-in" behavior (e.g. step-in doesn't step into code with no debug info), but also
the behavior of step-in/step-out and step-over when they step out of the frame they started in.
I also added as a proof of concept of this reworking a mode for stepping where stepping out of a frame
into a frame with no debug information will continue stepping out till it arrives at a frame that does
have debug information. This is useful when you are debugging callback based code where the callbacks
are separated from the code that initiated them by some library glue you don't care about, among other
things.
llvm-svn: 203747
