| Age | Commit message (Collapse) | Author |
|---|
|
In #129307, we introduced read write in chunks, and during the final
revision of the PR I changed the behavior for 64b memory regions and did
not test an actual 64b memory range.
This caused LLDB to crash whenever we generated a 64b memory region.
64b regions has been a problem in testing for some time as it's a waste
of test resources to generation a 5gb+ Minidump. I will work with
@clayborg and @labath to come up with a way to specify creating a 64b
list instead of a 32b list (likely via the yamilizer).
|
|
As per comment in https://github.com/llvm/llvm-project/pull/138698#issuecomment-2860369432
|
|
Add a generous amount of buffer directories. I found out some LLDB forks
(internal and external) had custom ranges that could fail because we
didn't pre-account for those. To prevent this from being a problem, I've
added a large number of buffer directories at the cost of 240 bytes.
|
|
I've recently been working on Minidump File Builder again, and some of
the comments are out of date, and many of the includes are no longer
used. This patch removes unneeded includes and rephrases some comments
to better fit with the current state after the read write chunks pr.
|
|
I recently received an internal error report that LLDB was OOM'ing when
creating a Minidump. In my 64b refactor we made a decision to acquire
buffers the size of the largest memory region so we could read all of
the contents in one call. This made error handling very simple (and
simpler coding for me!) but had the trade off of large allocations if
huge pages were enabled.
This patch is one I've had on the back burner for awhile, but we can
read and write the Minidump memory sections in discrete chunks which we
already do for writing to disk.
I had to refactor the error handling a bit, but it remains the same. We
make a best effort attempt to read as much of the memory region as
possible, but fail immediately if we receive an error writing to disk. I
did not add new tests for this because our existing test suite is quite
good, but I did manually verify a few Minidumps couldn't read beyond the
red_zone.
```
(lldb) reg read $sp
rsp = 0x00007fffffffc3b0
(lldb) p/x 0x00007fffffffc3b0 - 128
(long) 0x00007fffffffc330
(lldb) memory read 0x00007fffffffc330
0x7fffffffc330: 60 c3 ff ff ff 7f 00 00 60 cd ff ff ff 7f 00 00 `.......`.......
0x7fffffffc340: 60 c3 ff ff ff 7f 00 00 65 e6 26 00 00 00 00 00 `.......e.&.....
(lldb) memory read 0x00007fffffffc329
error: could not parse memory info (Success!)
```
I'm not sure how to quantify the memory improvement other than we would
allocate the largest size regardless of the size. So a 2gb unreadable
region would cause a 2gb allocation even if we were reading 4096 kb. Now
we will take the range size or the max chunk size of 128 mb.
|
|
In #119598 my recent TLS feature seems to break crashpad symbols. I have
a few ideas on how this is happening, but for now as a mitigation I'm
checking if the Minidump was LLDB generated, and if so leveraging the
dynamic loader.
|
|
Recently my coworker @jeffreytan81 pointed out that Minidumps don't show
breakpoints when collected. This was prior blocked because Minidumps
could only contain 1 exception, now that we support N signals/sections
we can save all the threads stopped on breakpoints.
|
|
stacks (#110579)
In my prior two save core API's, I experimented on how to save stacks
with the new API. I incorrectly left these in, as the existing
`m_thread_by_range_end` was the correct choice.
I have removed the no-op collection, and moved to use the proper one.
It's worth noting this was not caught by testing because we do not
verify where the items are contained in the minidump. This would require
a test being aware of how minidumps are structured, or adding a textual
tool that we can then scan the output of.
|
|
Added a progress tracker to Minidump file builders memory saving
|
|
In #95312 Minidump file creation was moved from being created at the
end, to the file being emitted in chunks. This causes some undesirable
behavior where the file can still be present after an error has
occurred. To resolve this we will now delete the file upon an error.
|
|
Recently in #107731 this change was revereted due to excess memory size
in `TestSkinnyCore`. This was due to a bug where a range's end was being
passed as size. Creating massive memory ranges.
Additionally, and requiring additional review, I added more unit tests
and more verbose logic to the merging of save core memory regions.
@jasonmolenda as an FYI.
|
|
Reverts llvm/llvm-project#107159 as this is still causing
`TestSkinnyCorefile.py` to time out.
https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/11099/
https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/lldb-cmake/5544/
|
|
Reapplies #106293, testing identified issue in the merging code. I used
this opportunity to strip CoreFileMemoryRanges to it's own file and then
add unit tests on it's behavior.
|
|
gs_base (#106767)
A follow up to #106473 Minidump wasn't collecting fs or gs_base. This
patch extends the x86_64 register context and gated reading it behind an
lldb specific flag. Additionally these registers are explicitly checked
in the tests.
|
|
file builder (#106473)
This patch addresses a bug where `cs`/`fs` and other segmentation flags
were being identified as having a type of `32b` and `64b` for `rflags`.
In that case the register value was returning the fail value `0xF...`
and this was corrupting some minidumps. Here we just read it as a 64b
value and truncate it.
In addition to that fix, I added comparing the registers from the live
process to the loaded core for the generic minidump test. Prior only
being ARM register tests. This explains why this was not detected
before.
|
|
(#106293)
Reverts #105442. Due to `TestSkinnyCoreFailing` and root causing of the
failure will likely take longer than EOD.
|
|
This patch removes all of the Set.* methods from Status.
This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.
This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()
Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form
` ResultTy DoFoo(Status& error)
`
to
` llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?
The interesting changes are in Status.h and Status.cpp, all other
changes are mostly
` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.
|
|
This patch adds the option to specify specific memory ranges to be
included in a given core file. The current implementation lets user
specified ranges either be in addition to a certain save style, or
independent of them via the newly added custom enum.
To achieve being inclusive of save style, I've moved from a std::vector
of ranges to a RangeDataVector, and to join overlapping ranges to
prevent duplication of memory ranges in the core file.
As a non function bonus, when SBSavecore was initially created, the
header was included in the lldb-private interfaces, and I've fixed that
and moved it the forward declare as an oversight. CC @bulbazord in case
we need to include that into swift.
|
|
regions when /proc/pid maps are missing. (#101086)
This PR is in response to a bug my coworker @mbucko discovered where on
MacOS Minidumps were being created where the 64b memory regions were
readable, but were not being listed in
`SBProcess.GetMemoryRegionList()`. This went unnoticed in #95312 due to
all the linux testing including /proc/pid maps. On MacOS generated dumps
(or any dump without access to /proc/pid) we would fail to properly map
Memory Regions due to there being two independent methods for 32b and
64b mapping.
In this PR I addressed this minor bug and merged the methods, but in
order to add test coverage required additions to `obj2yaml` and
`yaml2obj` which make up the bulk of this patch.
Lastly, there are some non-required changes such as the addition of the
`Memory64ListHeader` type, to make writing/reading the header section of
the Memory64List easier.
|
|
Reapply #100443 and #101770. These were originally reverted due to a
test failure and an MSAN failure. I changed the test attribute to
restrict to x86 (following the other existing tests). I could not
reproduce the test or the MSAN failure and no repo steps were provided.
|
|
(#102018)
… Options. (#100443)"
This reverts commit 3e4af616334eae532f308605b89ff158dd195180.
@adrian-prantl FYI
Reverts #100443
|
|
(#100443)
In #98403 I enabled the SBSaveCoreOptions object, which allows users via
the scripting API to define what they want saved into their core file.
As the first option I've added a threadlist, so users can scan and
identify which threads and corresponding stacks they want to save.
In order to support this, I had to add a new method to `Process.h` on
how we identify which threads are to be saved, and I had to change the
book keeping in minidump to ensure we don't double save the stacks.
Important to @jasonmolenda I also changed the MachO coredump to accept
these new APIs.
|
|
(#96564)
In #95312 I incorrectly set `m_expected_directories` to uint, this broke
the windows build and is the incorrect type.
`size_t` is more accurate because this value only ever represents the
expected upper bound of the directory vector.
|
|
Currently, LLDB does not support taking a minidump over the 4.2gb limit imposed by uint32. In fact, currently it writes the RVA's and the headers to the end of the file, which can become corrupted due to the header offset only supporting a 32b offset.
This change reorganizes how the file structure is laid out. LLDB will precalculate the number of directories required and preallocate space at the top of the file to fill in later. Additionally, thread stacks require a 32b offset, and we provision empty descriptors and keep track of them to clean up once we write the 32b memory list.
For
[MemoryList64](https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_memory64_list),
the RVA to the start of the section itself will remain in a 32b addressable space. We achieve this by predetermining the space the memory regions will take, and only writing up to 4.2 gb of data with some buffer to allow all the MemoryDescriptor64s to also still be 32b addressable.
I did not add any explicit tests to this PR because allocating 4.2gb+ to test is very expensive. However, we have 32b automation tests and I validated with in several ways, including with 5gb+ array/object and would be willing to add this as a test case.
|
|
red_zone (#92002)
Currently in Core dumps, the entire pthread is copied, including the
unused space beyond the stack pointer. This causes large amounts of core
dump inflation when the number of threads is high, but the stack usage
is low. Such as when an application is using a thread pool.
This change will optimize for these situations in addition to generally
improving the core dump performance for all of lldb.
|
|
Summary:
AddMemoryList() was returning the last error status returned by
ReadMemory(). So if an invalid memory region was read last, the function
would return an error.
Test Plan:
./bin/llvm-lit -sv
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
Reviewers:
kevinfrei,clayborg
Subscribers:
Tasks:
Tags:
|
|
There are users reporting saving minidump from lldb-dap does not work.
Turns out our stack trace request always evaluate a function call which
caused JIT object file like "__lldb_caller_function" to be created which
can fail minidump builder to get its module size.
This patch fixes "getModuleFileSize" for ObjectFileJIT so that module
list can be saved. I decided to create a lldb-dap test so that this
end-to-end functionality can be validated from our tests (instead of
only command line lldb).
The patch also improves several other small things in the workflow:
1. It logs any minidump stream failure so that it is easier to find out
what stream saving fails. In future, we should show process and error to
end users.
2. It handles error from "getModuleFileSize" llvm::Expected<T> otherwise
it will complain the error is not handled.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
|
|
This patch adds support for saving minidumps with the arm64
architecture. It also will cause unsupported architectures to emit an
error where before this patch it would emit a minidump with partial
information. This new code is tested by the arm64 windows buildbot that
was failing:
https://lab.llvm.org/buildbot/#/builders/219/builds/6868
This is needed following this PR:
https://github.com/llvm/llvm-project/pull/71772
|
|
files (#71772)
Prior to this patch, each core file plugin (ObjectFileMachO.cpp and
ObjectFileMinindump.cpp) would calculate the address ranges to save in
different ways. This patch adds a new function to Process.h/.cpp:
```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, CoreFileMemoryRanges &ranges);
```
The patch updates the ObjectFileMachO::SaveCore(...) and
ObjectFileMinindump::SaveCore(...) to use same code. This will allow
core files to be consistent with the lldb::SaveCoreStyle across
different core file creators and will allow us to add new core file
saving features that do more complex things in future patches.
|
|
Improve LLDB reliability by fixing the following "uninitialized variables" static code inspection warnings from
scan.coverity.com:
1476275, 1274012, 1455035, 1364789, 1454282
1467483, 1406152, 1406255, 1454837, 1454416
1467446, 1462022, 1461909, 1420566, 1327228
1367767, 1431254, 1467299, 1312678, 1431780
1454731, 1490403
Differential Revision: https://reviews.llvm.org/D130528
|
|
scan. Part 2"
This reverts commit b9aedd94e6796e4b4866ab4c091b736b3db58cb7.
|
|
Improve LLDB reliability by fixing the following "uninitialized variables" static code inspection warnings from
scan.coverity.com:
1476275, 1274012, 1455035, 1364789, 1454282
1467483, 1406152, 1406255, 1454837, 1454416
1467446, 1462022, 1461909, 1420566, 1327228
1367767, 1431254, 1467299, 1312678, 1431780
1454731, 1490403
Differential Revision: https://reviews.llvm.org/D130528
|
|
Improve LLDB reliability by fixing the following "uninitialized variables" static code inspection warnings from
scan.coverity.com:
1094796 1095721 1095728 1095737 1095741
1095756 1095779 1095789 1095805 1214552
1229457 1232475 1274006 1274010 1293427
1364800 1364802 1364804 1364812 1364816
1374902 1374909 1384975 1399312 1420451
1431704 1454230 1454554 1454615 1454579
1454594 1454832 1457759 1458696 1461909
1467658 1487814 1487830 1487845
Differential Revision: https://reviews.llvm.org/D130098
|
|
This is a re-submission of 24d240558811604354a8d6080405f6bad8d15b5c
without the hunks in HostNativeThreadBase.{h,cpp}, which break builds
on Windows.
Identified with modernize-use-nullptr.
|
|
This reverts commit 913457acf07be7f22d71ac41ad1076517d7f45c6.
It again broke builds on Windows:
lldb/source/Host/common/HostNativeThreadBase.cpp(37,14): error:
assigning to 'lldb::thread_result_t' (aka 'unsigned int') from
incompatible type 'std::nullptr_t'
|
|
This is a re-submission of 24d240558811604354a8d6080405f6bad8d15b5c
without the hunk in HostNativeThreadBase.h, which breaks builds on
Windows.
Identified with modernize-use-nullptr.
|
|
This reverts commit 24d240558811604354a8d6080405f6bad8d15b5c.
Breaks building on Windows:
../../lldb/include\lldb/Host/HostNativeThreadBase.h(49,36): error:
cannot initialize a member subobject of type 'lldb::thread_result_t'
(aka 'unsigned int') with an rvalue of type 'std::nullptr_t'
lldb::thread_result_t m_result = nullptr;
^~~~~~~
1 error generated.
|
|
Identified with modernize-use-nullptr.
|
|
Identified with readability-redundant-string-init.
|
|
And revert c4fa2c8aa493e4c786446739ba3eb0eb4918d579
|
|
Fixes the following warning:
$llvm_project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:744:11: warning:
format specifies type 'long' but the argument has type 'lldb::offset_t' (aka 'unsigned long long') [-Wformat]
m_data.GetByteSize());
^~~~~~~~~~~~~~~~~~~~
|
|
This change adds save-core functionality into the ObjectFileELF that enables
saving minidump of a stopped process. This change is mainly targeting Linux
running on x86_64 machines. Minidump should contain basic information needed
to examine state of threads, local variables and stack traces. Full support
for other platforms is not so far implemented. API tests are using LLDB's
MinidumpParser.
This relands commit aafa05e, reverted in 1f986f6.
Failed tests were fixed.
Reviewed By: clayborg
Differential Revision: https://reviews.llvm.org/D108233
|
|
This reverts commit aafa05e03d629cc6605718c54575256d9d683659.
Broke builder on aarch64 --
https://lab.llvm.org/buildbot/#/builders/96/builds/10926
|
|
This change adds save-core functionality into the ObjectFileELF that enables
saving minidump of a stopped process. This change is mainly targeting Linux
running on x86_64 machines. Minidump should contain basic information needed
to examine state of threads, local variables and stack traces. Full support
for other platforms is not so far implemented. API tests are using LLDB's
MinidumpParser.
Reviewed By: clayborg
Differential Revision: https://reviews.llvm.org/D108233
|
