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
Just delete the memset as the ELFHeader constructor already
zero-initializes the object. Also clean up the ObjectFileELF
constructors/desctructors while I'm in there.
llvm-svn: 366692
With this style, a compressed section is indicated by a "z" in the section
name, instead of a section header flag. This patch consists of two small tweaks:
- use an llvm Decompressor method in order to properly detect compressed sections
- make sure we recognise .zdebug_info (and friends) when classifying section types.
llvm-svn: 365654
Summary:
NFC = [[ https://llvm.org/docs/Lexicon.html#nfc | Non functional change ]]
This commit is the result of modernizing the LLDB codebase by using
`nullptr` instread of `0` or `NULL`. See
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html
for more information.
This is the command I ran and I to fix and format the code base:
```
run-clang-tidy.py \
-header-filter='.*' \
-checks='-*,modernize-use-nullptr' \
-fix ~/dev/llvm-project/lldb/.* \
-format \
-style LLVM \
-p ~/llvm-builds/debug-ninja-gcc
```
NOTE: There were also changes to `llvm/utils/unittest` but I did not
include them because I felt that maybe this library shall be updated in
isolation somehow.
NOTE: I know this is a rather large commit but it is a nobrainer in most
parts.
Reviewers: martong, espindola, shafik, #lldb, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arsenm, jvesely, nhaehnle, hiraditya, JDevlieghere, teemperor, rnkovacs, emaste, kubamracek, nemanjai, ki.stfu, javed.absar, arichardson, kbarton, jrtc27, MaskRay, atanasyan, dexonsmith, arphaman, jfb, jsji, jdoerfert, lldb-commits, llvm-commits
Tags: #lldb, #llvm
Differential Revision: https://reviews.llvm.org/D61847
llvm-svn: 361484
Summary:
First part of a fix for JITed code debugging. This has been a regression from 5.0 to 6.0 and it's is still reproducible on current master: https://bugs.llvm.org/show_bug.cgi?id=36209
The address of the breakpoint site is corrupt: the 0x4 value we end up with, looks like an offset on a zero base address. When we parse the ELF section headers from the JIT descriptor, the load address for the text section we find in `header.sh_addr` is correct.
The bug manifests in `VMAddressProvider::GetVMRange(const ELFSectionHeader &)` (follow it from `ObjectFileELF::CreateSections()`). Here we think the object type was `eTypeObjectFile` and unleash some extra logic [1] which essentially overwrites the address with a zero value.
The object type is deduced from the ELF header's `e_type` in `ObjectFileELF::CalculateType()`. It never returns `eTypeJIT`, because the ELF header has no representation for it [2]. Instead the in-memory ELF object states `ET_REL`, which leads to `eTypeObjectFile`. This is what we get from `lli` at least since 3.x. (Might it be better to write `ET_EXEC` on the JIT side instead? In fact, relocations were already applied at this point, so "Relocatable" is not quite exact.)
So, this patch proposes to set `eTypeJIT` explicitly whenever we read from a JIT descriptor. In `ObjectFileELF::CreateSections()` we can then call `GetType()`, which returns the explicit value or otherwise falls back to `CalculateType()`.
LLDB then sets the breakpoint successfully. Next step: debug info.
```
Process 1056 stopped
* thread #1, name = 'lli', stop reason = breakpoint 1.2
frame #0: 0x00007ffff7ff7000 JIT(0x3ba2030)`jitbp()
JIT(0x3ba2030)`jitbp:
-> 0x7ffff7ff7000 <+0>: pushq %rbp
0x7ffff7ff7001 <+1>: movq %rsp, %rbp
0x7ffff7ff7004 <+4>: movabsq $0x7ffff7ff6000, %rdi ; imm = 0x7FFFF7FF6000
0x7ffff7ff700e <+14>: movabsq $0x7ffff6697e80, %rcx ; imm = 0x7FFFF6697E80
```
[1] It was first introduced with https://reviews.llvm.org/D38142#change-lF6csxV8HdlL, which has also been the original breaking change. The code has changed a lot since then.
[2] ELF object types: https://github.com/llvm/llvm-project/blob/2d2277f5/llvm/include/llvm/BinaryFormat/ELF.h#L110
Reviewers: labath, JDevlieghere, bkoropoff, clayborg, espindola, alexshap, stella.stamenova
Reviewed By: labath, clayborg
Subscribers: probinson, emaste, aprantl, arichardson, MaskRay, AlexDenisov, yurydelendik, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61611
llvm-svn: 360354
Summary:
When we want to compare a ConstString against a string literal (or any other non-ConstString),
we currently have to explicitly turn the other string into a ConstString. This makes sense as
comparing ConstStrings against each other is only a fast pointer comparison.
However, currently we (rather incorrectly) use in several places in LLDB temporary ConstStrings when
we just want to compare a given ConstString against a hardcoded value, for example like this:
```
if (extension != ConstString(".oat") && extension != ConstString(".odex"))
```
Obviously this kind of defeats the point of ConstStrings. In the comparison above we would
construct two temporary ConstStrings every time we hit the given code. Constructing a
ConstString is relatively expensive: we need to go to the StringPool, take a read and possibly
an exclusive write-lock and then look up our temporary string in the string map of the pool.
So we do a lot of heavy work for essentially just comparing a <6 characters in two strings.
I initially wanted to just fix these issues by turning the temporary ConstString in static variables/
members, but that made the code much less readable. Instead I propose to add a new overload
for the ConstString comparison operator that takes a StringRef. This comparison operator directly
compares the ConstString content against the given StringRef without turning the StringRef into
a ConstString.
This means that the example above can look like this now:
```
if (extension != ".oat" && extension != ".odex")
```
It also no longer has to unlock/lock two locks and call multiple functions in other TUs for constructing
the temporary ConstString instances. Instead this should end up just being a direct string comparison
of the two given strings on most compilers.
This patch also directly updates all uses of temporary and short ConstStrings in LLDB to use this new
comparison operator. It also adds a some unit tests for the new and old comparison operator.
Reviewers: #lldb, JDevlieghere, espindola, amccarth
Reviewed By: JDevlieghere, amccarth
Subscribers: amccarth, clayborg, JDevlieghere, emaste, arichardson, MaskRay, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D60667
llvm-svn: 359281
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
My apologies for the large patch. With the exception of ConstString.h
itself it was entirely produced by sed.
ConstString has exactly one const char * data member, so passing a
ConstString by reference is not any more efficient than copying it by
value. In both cases a single pointer is passed. But passing it by
value makes it harder to accidentally return the address of a local
object.
(This fixes rdar://problem/48640859 for the Apple folks)
Differential Revision: https://reviews.llvm.org/D59030
llvm-svn: 355553
Summary:
This file implements some general purpose data structures, and so it
belongs to the Utility module.
Reviewers: zturner, jingham, JDevlieghere, clayborg, espindola
Subscribers: emaste, mgorny, javed.absar, arichardson, MaskRay, lldb-commits
Differential Revision: https://reviews.llvm.org/D58970
llvm-svn: 355509
Summary:
These functions should always return the opposite of the
`Triple{Environment,OS,Vendor}WasSpecified` functions. Unspecified unknown is
the same as unspecified, which is why one set of functions should give us what
we want. It's possible to have specified unknown, which is why we can't just
rely on checking the enum values of vendor/os/environment. We must also ensure
that the names of these are empty and not "unknown".
Differential Revision: https://reviews.llvm.org/D58653
llvm-svn: 354933
Split the recognition into NetBSD executables & shared libraries
and core(5) files.
Introduce new owner type: "NetBSD-CORE", as core(5) files are not tagged
in the same way as regular NetBSD executables.
Stop using incorrectly ABI_TAG and ABI_SIZE. Introduce IDENT_TAG,
IDENT_DECSZ, IDENT_NAMESZ and PROCINFO.
The new values detect correctly the NetBSD images.
The patch has been originally written by Kamil Rytarowski. I've added
tests and applied minor code changes per review. The work has been
sponsored by the NetBSD Foundation.
Differential Revision: https://reviews.llvm.org/D42870
llvm-svn: 354466
Summary:
This is a preparatory step to enable adding extra unwind strategies by
symbol file plugins. This has been discussed on the lldb-dev mailing
list: <http://lists.llvm.org/pipermail/lldb-dev/2019-February/014703.html>.
Reviewers: jasonmolenda, clayborg, espindola
Subscribers: lemo, emaste, lldb-commits, arichardson
Differential Revision: https://reviews.llvm.org/D58129
llvm-svn: 354033
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
instead of returning the UUID through by-ref argument and a boolean
value indicating success, we can just return it directly. Since the UUID
class already has an invalid state, it can be used to denote the failure
without the additional bool.
llvm-svn: 353714
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
The code was assuming that the elf file will have a PT_LOAD segment
starting from the first byte of the file. While this is true for files
generated by most linkers (it's a way of saving space), it is not a
requirement. And files not satisfying this constraint can still be
perfectly executable. yaml2obj is one of the tools which produces files
like this.
This patch relaxes the check in ObjectFileELF to take the address of the
first PT_LOAD segment as the base address of the object (instead of the
one with the offset 0). Since the PT_LOAD segments are supposed to be
sorted according to the VM address, this entry will also be the one with
the lowest VM address.
If we ever run into files which don't have the PT_LOAD segments sorted,
we can easily change this code to return the lowest VM address as the
base address (if that is the correct thing to do for these files).
llvm-svn: 350923
Summary:
The concept of a base address was already present in the implementation
(it's needed for computing section load addresses properly), but it was
never exposed through this function. This fixes that.
llvm-svn: 350804
Summary:
This is the result of the discussion in D55356, where it was suggested
as a solution to representing the addresses that logically belong to a
module in memory, but are not a part of any of its sections.
The ELF PT_LOAD segments are similar to the MachO "load commands",
except that the relationship between them and the object file sections
is a bit weaker. While in the MachO case, the sections belonging to a
specific segment are placed directly inside it in the object file
logical structur, in the ELF case, the sections and segments form two
separate hierarchies. This means that it is in theory possible to create
an elf file where only a part of a section would belong to some segment
(and another part to a different one). However, I am not aware of any
tool which would produce such a file (and most tools will have problems
ingesting them), so this means it is still possible to follow the MachO
model and make sections children of the PT_LOAD segments.
In case we run into (corrupt?) files with overlapping sections, I have
added code (and tests) which adjusts the sizes and/or drops the offending
sections in order to present a reasonable image to the upper layers of
LLDB. This is mostly done for completeness, as I don't anticipate
running into this situation in the real world. However, if we do run
into it, and the current behavior is not suitable for some reason, we
can implement this logic differently.
Reviewers: clayborg, jankratochvil, krytarowski, joerg, espindola
Subscribers: emaste, arichardson, lldb-commits
Differential Revision: https://reviews.llvm.org/D55998
llvm-svn: 350742
Summary:
instead of returning the architecture through by-ref argument and a
boolean value indicating success, we can just return the ArchSpec
directly. Since the ArchSpec already has an invalid state, it can be
used to denote the failure without the additional bool.
Reviewers: clayborg, zturner, espindola
Subscribers: emaste, arichardson, JDevlieghere, lldb-commits
Differential Revision: https://reviews.llvm.org/D56129
llvm-svn: 350291