[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: @petrhosek I am working on this. I think it's only the header thankfully https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
petrhosek wrote: We're seeing the same issue as well on our builders, will you be able to address this today? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
vvereschaka wrote: @Jlalond , perfect, thank you. It is ok for me if you'll fix the problem during today. https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: @vvereschaka I'll work on this today. Is it time pressing enough you would want everything reverted? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
vvereschaka wrote: Hi @Jlalond , here is still a problem with the window builds: ``` FAILED: tools/lldb/source/Plugins/ObjectFile/Minidump/CMakeFiles/lldbPluginObjectFileMinidump.dir/ObjectFileMinidump.cpp.obj ccache C:\PROGRA~1\LLVM\bin\clang-cl.exe /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source\Plugins\ObjectFile\Minidump -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Plugins\ObjectFile\Minidump -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include -IC:\Users\tcwg\AppData\Local\Programs\Python\Python311-arm64\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source /DWIN32 /D_WINDOWS /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- -Werror=unguarded-availability-new /W4 -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -Wno-deprecated-register -Wno-vla-extension /O2 /Ob2 /DNDEBUG -MD -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- -std:c++17 /showIncludes /Fotools\lldb\source\Plugins\ObjectFile\Minidump\CMakeFiles\lldbPluginObjectFileMinidump.dir\ObjectFileMinidump.cpp.obj /Fdtools\lldb\source\Plugins\ObjectFile\Minidump\CMakeFiles\lldbPluginObjectFileMinidump.dir\lldbPluginObjectFileMinidump.pdb -c -- C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Plugins\ObjectFile\Minidump\ObjectFileMinidump.cpp C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Plugins\ObjectFile\Minidump\ObjectFileMinidump.cpp(21,10): fatal error: 'unistd.h' file not found 21 | #include | ^~ 1 error generated. ``` * https://lab.llvm.org/buildbot/#/builders/141/builds/266 * https://lab.llvm.org/buildbot/#/builders/141/builds/250 https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: Fixed uint issue in #96564 https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: @petrhosek because it's just going to be a change to something like `size_t`, would you mind if I just did a quick commit to fix this and assign you as the reviewer? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
petrhosek wrote: This broke the Windows build with the following error: ``` C:\b\s\w\ir\x\w\cipd\bin\clang-cl.exe /nologo -TP -DGTEST_HAS_RTTI=0 -DLIBXML_STATIC -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\b\s\w\ir\x\w\llvm_build\tools\lldb\source\Plugins\ObjectFile\Minidump -IC:\b\s\w\ir\x\w\llvm-llvm-project\lldb\source\Plugins\ObjectFile\Minidump -IC:\b\s\w\ir\x\w\llvm-llvm-project\lldb\include -IC:\b\s\w\ir\x\w\llvm_build\tools\lldb\include -IC:\b\s\w\ir\x\w\rc\tensorflow-venv\store\python_venv-51qj4upsi8nrslpsnfp48k5j14\contents\Lib\site-packages\tensorflow\include -IC:\b\s\w\ir\x\w\llvm_build\include -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\include -IC:\b\s\w\ir\x\w\lldb_install\python3\include -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\..\clang\include -IC:\b\s\w\ir\x\w\llvm_build\tools\lldb\..\clang\include -IC:\b\s\w\ir\x\w\llvm-llvm-project\lldb\source -IC:\b\s\w\ir\x\w\llvm_build\tools\lldb\source -imsvcC:\b\s\w\ir\x\w\libxml2_install_target\include\libxml2 -imsvcC:\b\s\w\ir\x\w\zlib_install_target\include -imsvcC:\b\s\w\ir\x\w\zstd_install\include /DWIN32 /D_WINDOWS /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- -Werror=unguarded-availability-new /W4 -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -Wno-deprecated-register -Wno-vla-extension /O2 /Ob2 -std:c++17 -MT -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- -UNDEBUG /showIncludes /Fotools\lldb\source\Plugins\ObjectFile\Minidump\CMakeFiles\lldbPluginObjectFileMinidump.dir\MinidumpFileBuilder.cpp.obj /Fdtools\lldb\source\Plugins\ObjectFile\Minidump\CMakeFiles\lldbPluginObjectFileMinidump.dir\lldbPluginObjectFileMinidump.pdb -c -- C:\b\s\w\ir\x\w\llvm-llvm-project\lldb\source\Plugins\ObjectFile\Minidump\MinidumpFileBuilder.cpp In file included from C:\b\s\w\ir\x\w\llvm-llvm-project\lldb\source\Plugins\ObjectFile\Minidump\MinidumpFileBuilder.cpp:9: C:\b\s\w\ir\x\w\llvm-llvm-project\lldb\source\Plugins\ObjectFile\Minidump\MinidumpFileBuilder.h(149,3): error: unknown type name 'uint'; did you mean 'int'? 149 | uint m_expected_directories = 0; | ^~~~ | int 1 error generated. ``` Would it be possible to take a look and revert if necessary? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg approved this pull request. Looks good to me! https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +807,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ // We first save the thread stacks to ensure they fit in the first UINT32_MAX
+ // bytes of the core file. Thread structures in minidump files can only use
+ // 32 bit memory descriptiors, so we emit them first to ensure the memory is
+ // in accessible with a 32 bit offset.
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // After saving the stacks, we start packing as much as we can into 32b.
+ // We apply a generous padding here so that the Directory, MemoryList and
+ // Memory64List sections all begin in 32b addressable space.
+ // Then anything overflow extends into 64b addressable space.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
Jlalond wrote:
This actually works, because we'll never enter the loop unless
`all_core_memory_ranges` is non empty, where total_size will actually be used.
Prior to code review feedback we always called to get memory ranges, even when
it's just stacks, ensuring we would always have at least stacks in
`all_core_memory_ranges`.
I have added the check for empty and an explanation comment as well.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +807,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ // We first save the thread stacks to ensure they fit in the first UINT32_MAX
+ // bytes of the core file. Thread structures in minidump files can only use
+ // 32 bit memory descriptiors, so we emit them first to ensure the memory is
+ // in accessible with a 32 bit offset.
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // After saving the stacks, we start packing as much as we can into 32b.
+ // We apply a generous padding here so that the Directory, MemoryList and
+ // Memory64List sections all begin in 32b addressable space.
+ // Then anything overflow extends into 64b addressable space.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t range_size = core_range.range.size();
clayborg wrote:
make `const`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +807,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ // We first save the thread stacks to ensure they fit in the first UINT32_MAX
+ // bytes of the core file. Thread structures in minidump files can only use
+ // 32 bit memory descriptiors, so we emit them first to ensure the memory is
+ // in accessible with a 32 bit offset.
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // After saving the stacks, we start packing as much as we can into 32b.
+ // We apply a generous padding here so that the Directory, MemoryList and
+ // Memory64List sections all begin in 32b addressable space.
+ // Then anything overflow extends into 64b addressable space.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
clayborg wrote:
this `total_size` will be wrong if `all_core_memory_ranges` is empty right? If
we are saving stacks only, this will be:
```
256 + (0 - stack_start_addresses.size()) *
sizeof(llvm::minidump::MemoryDescriptor_64);
```
And we will unsigned underflow but it won't matter because we won't use
`total_size`. Best to put a:
```
if (!all_core_memory_ranges.empty()) {
```
around the `total_size` and for loop that iterates over all core ranges to
avoid potential issues in the future.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -20,25 +20,98 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RangeMap.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size = HEADER_SIZE;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
+ lldb_private::Target = m_process_sp->GetTarget();
+ m_expected_directories = 6;
+ // Check if OS is linux and reserve directory space for all linux specific
+ // breakpad extension directories.
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp &&
+stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ m_saved_data_size +=
+ m_expected_directories * sizeof(llvm::minidump::Directory);
+ Status error;
+ offset_t new_offset = m_core_file->SeekFromStart(m_saved_data_size);
+ if (new_offset != m_saved_data_size)
+error.SetErrorStringWithFormat("Failed to fill in header and directory "
+ "sections. Written / Expected (%" PRIx64
+ " / %zu)",
clayborg wrote:
`m_saved_data_size` is uint64_t, so don't use `" / %zu)`, use ` / %" PRIu64 ")"
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +807,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ // We first save the thread stacks to ensure they fit in the first UINT32_MAX
+ // bytes of the core file. Thread structures in minidump files can only use
+ // 32 bit memory descriptiors, so we emit them first to ensure the memory is
+ // in accessible with a 32 bit offset.
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // After saving the stacks, we start packing as much as we can into 32b.
+ // We apply a generous padding here so that the Directory, MemoryList and
+ // Memory64List sections all begin in 32b addressable space.
+ // Then anything overflow extends into 64b addressable space.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t range_size = core_range.range.size();
+if (stack_start_addresses.count(core_range.range.start()) > 0)
+ // Don't double save stacks.
+ continue;
+
+if (total_size + range_size < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
clayborg wrote:
use `range_size` that you created above.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges)
+max_size = std::max(max_size, core_range.range.size());
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges)
+max_size = std::max(max_size, core_range.range.size());
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +805,99 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
clayborg wrote:
Remove `sizeof(llvm::minidump::MemoryDescriptor)` from here since we account
for it before this for loop. If this is now just the core range size, rename
like `range_size`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges)
+max_size = std::max(max_size, core_range.range.size());
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -480,71 +559,64 @@ class ArchThreadContexts {
}
};
-// Function returns start and size of the memory region that contains
-// memory location pointed to by the current stack pointer.
-llvm::Expected>
-findStackHelper(const lldb::ProcessSP _sp, uint64_t rsp) {
- MemoryRegionInfo range_info;
- Status error = process_sp->GetMemoryRegionInfo(rsp, range_info);
- // Skip failed memory region requests or any regions with no permissions.
- if (error.Fail() || range_info.GetLLDBPermissions() == 0)
-return llvm::createStringError(
-std::errc::not_supported,
-"unable to load stack segment of the process");
-
- // This is a duplicate of the logic in
- // Process::SaveOffRegionsWithStackPointers but ultimately, we need to only
- // save up from the start of the stack down to the stack pointer
- const addr_t range_end = range_info.GetRange().GetRangeEnd();
- const addr_t red_zone = process_sp->GetABI()->GetRedZoneSize();
- const addr_t stack_head = rsp - red_zone;
- if (stack_head > range_info.GetRange().GetRangeEnd()) {
-range_info.GetRange().SetRangeBase(stack_head);
-range_info.GetRange().SetByteSize(range_end - stack_head);
- }
-
- const addr_t addr = range_info.GetRange().GetRangeBase();
- const addr_t size = range_info.GetRange().GetByteSize();
-
- if (size == 0)
-return llvm::createStringError(std::errc::not_supported,
- "stack segment of the process is empty");
-
- return std::make_pair(addr, size);
+Status MinidumpFileBuilder::FixThreadStacks() {
+ Status error;
+ // If we have anything in the heap flush it.
+ FlushBufferToDisk();
+ m_core_file->SeekFromStart(m_thread_list_start);
+ for (auto : m_thread_by_range_end) {
+// The thread objects will get a new memory descriptor added
+// When we are emitting the memory list and then we write it here
+const llvm::minidump::Thread = pair.second;
+size_t bytes_to_write = sizeof(llvm::minidump::Thread);
+size_t bytes_written = bytes_to_write;
+error = m_core_file->Write(, bytes_written);
clayborg wrote:
We can do this in a later patch as the `sizeof(llvm::minidump::Thread)` won't
exceed 32 bits
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges)
+max_size = std::max(max_size, core_range.range.size());
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
clayborg wrote:
Change `"(%zu bytes) [%zx, %zx)"` to `"(%" PRIu64 " bytes [%" PRIx64 ", %"
PRIx64 ")"`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges)
+max_size = std::max(max_size, core_range.range.size());
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
clayborg wrote:
uint64_t
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
clayborg wrote:
remove static_casts
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
clayborg wrote:
use `uint64_t`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -821,47 +908,285 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
Status error;
size_t bytes_written;
- bytes_written = header_size;
- error = core_file->Write(, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ m_core_file->SeekFromStart(0);
+ bytes_written = HEADER_SIZE;
+ error = m_core_file->Write(, bytes_written);
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, HEADER_SIZE);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
-if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
-return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory : m_directories) {
-bytes_written = directory_size;
-error = core_file->Write(, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+bytes_written = DIRECTORY_SIZE;
+error = m_core_file->Write(, bytes_written);
+if (error.Fail() || bytes_written != DIRECTORY_SIZE) {
+ if (bytes_written != DIRECTORY_SIZE)
error.SetErrorStringWithFormat(
"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
+DIRECTORY_SIZE);
return error;
}
}
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
clayborg wrote:
Rename to match what it is returning and don't use `size_t`, this will fail on
32 bit systems if we have a range over UINT32_MAX:
```
static uint64_t GetLargestRangeSize(const Process::CoreFileMemoryRanges
) {
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +805,99 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0)
+ // Don't double save stacks.
+ continue;
+
+if (total_size + size_to_add < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
+} else {
+ ranges_64.push_back(core_range);
+}
+ }
+
+ error = AddMemoryList_32(ranges_32);
+ if (error.Fail())
+return error;
+
+ // Add the remaining memory as a 64b range.
+ if (!ranges_64.empty()) {
+error = AddMemoryList_64(ranges_64);
+if (error.Fail())
+ return error;
+ }
+ return FixThreadStacks();
+}
+
+Status MinidumpFileBuilder::DumpHeader() const {
// write header
llvm::minidump::Header header;
header.Signature = static_cast(
llvm::minidump::Header::MagicSignature);
header.Version = static_cast(
llvm::minidump::Header::MagicVersion);
header.NumberOfStreams =
- static_cast(GetDirectoriesNum());
+ static_cast(m_directories.size());
+ // We write the directories right after the header.
header.StreamDirectoryRVA =
- static_cast(GetCurrentDataEndOffset());
+ static_cast(HEADER_SIZE);
clayborg wrote:
remove static_cast, not needed.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
+}
+
+Status MinidumpFileBuilder::AddMemoryList_64(
+Process::CoreFileMemoryRanges ) {
+ Status error;
+ if (ranges.empty())
+return error;
+
+ error = AddDirectory(StreamType::Memory64List,
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast(ranges.size());
clayborg wrote:
remove static casts
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -480,71 +559,64 @@ class ArchThreadContexts {
}
};
-// Function returns start and size of the memory region that contains
-// memory location pointed to by the current stack pointer.
-llvm::Expected>
-findStackHelper(const lldb::ProcessSP _sp, uint64_t rsp) {
- MemoryRegionInfo range_info;
- Status error = process_sp->GetMemoryRegionInfo(rsp, range_info);
- // Skip failed memory region requests or any regions with no permissions.
- if (error.Fail() || range_info.GetLLDBPermissions() == 0)
-return llvm::createStringError(
-std::errc::not_supported,
-"unable to load stack segment of the process");
-
- // This is a duplicate of the logic in
- // Process::SaveOffRegionsWithStackPointers but ultimately, we need to only
- // save up from the start of the stack down to the stack pointer
- const addr_t range_end = range_info.GetRange().GetRangeEnd();
- const addr_t red_zone = process_sp->GetABI()->GetRedZoneSize();
- const addr_t stack_head = rsp - red_zone;
- if (stack_head > range_info.GetRange().GetRangeEnd()) {
-range_info.GetRange().SetRangeBase(stack_head);
-range_info.GetRange().SetByteSize(range_end - stack_head);
- }
-
- const addr_t addr = range_info.GetRange().GetRangeBase();
- const addr_t size = range_info.GetRange().GetByteSize();
-
- if (size == 0)
-return llvm::createStringError(std::errc::not_supported,
- "stack segment of the process is empty");
-
- return std::make_pair(addr, size);
+Status MinidumpFileBuilder::FixThreadStacks() {
+ Status error;
+ // If we have anything in the heap flush it.
+ FlushBufferToDisk();
+ m_core_file->SeekFromStart(m_thread_list_start);
+ for (auto : m_thread_by_range_end) {
+// The thread objects will get a new memory descriptor added
+// When we are emitting the memory list and then we write it here
+const llvm::minidump::Thread = pair.second;
+size_t bytes_to_write = sizeof(llvm::minidump::Thread);
+size_t bytes_written = bytes_to_write;
+error = m_core_file->Write(, bytes_written);
clayborg wrote:
We should probably change the underlying `Write(...)` function to not use
`size_t` if it is as this will fail on 32 bit architectures, or we need to
write at most SIZET_MAX
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +805,99 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set stack_start_addresses;
+ for (const auto _range : ranges_32) {
+stack_start_addresses.insert(core_range.range.start());
+total_size += core_range.range.size();
+ }
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0)
+ // Don't double save stacks.
+ continue;
+
+if (total_size + size_to_add < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
+} else {
+ ranges_64.push_back(core_range);
+}
+ }
+
+ error = AddMemoryList_32(ranges_32);
+ if (error.Fail())
+return error;
+
+ // Add the remaining memory as a 64b range.
+ if (!ranges_64.empty()) {
+error = AddMemoryList_64(ranges_64);
+if (error.Fail())
+ return error;
+ }
+ return FixThreadStacks();
+}
+
+Status MinidumpFileBuilder::DumpHeader() const {
// write header
llvm::minidump::Header header;
header.Signature = static_cast(
llvm::minidump::Header::MagicSignature);
header.Version = static_cast(
llvm::minidump::Header::MagicVersion);
header.NumberOfStreams =
- static_cast(GetDirectoriesNum());
+ static_cast(m_directories.size());
clayborg wrote:
remove static_cast, not needed.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
clayborg wrote:
The assignment operator can and must work for these
`llvm::support::ulittleXX_t` values, we should take advantage of them and keep
the code cleaner. Removing all static_casts makes this code easier to read and
avoids people making copy/paste of similar code in the future. I know there
were some static casts before this patch, but we should remove them.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +805,99 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
clayborg wrote:
Add a comment here:
```
// We first save the thread stacks to ensure they fit in the first UINT32_MAX
// bytes of the core file. Thread structures in minidump files can only use
// 32 bit memory descriptiors, so we emit them first to ensure the memory is
// in accessible with a 32 bit offset.
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -20,25 +20,96 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RangeMap.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += HEADER_SIZE;
clayborg wrote:
Should't this be:
```
m_saved_data_size = HEADER_SIZE;
```
There is no need to use `+=` right?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
Jlalond wrote:
Messaged offline, but it doesn't appear all the little endian types support
conversion. Specifically ulittle64_t and ulittle_t for the stream type. I think
we should fix this in a followup, and add a formatter for little endian types.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +923,224 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
Jlalond wrote:
Done. Albeit the `m_data` buffer that the class uses does get used and then
reallocated when we call `Clear()` in flush to disk. May want to look into
keeping a write index and a range...
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
Jlalond wrote:
I'm torn on the static casts. I know the little endian support types can
support big endian assignment, but I think the static cast makes it very
explicit that these values have to be little endian
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +812,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+
+ std::set stack_start_addresses;
+ for (const auto _range : ranges_32)
+stack_start_addresses.insert(core_range.range.start());
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto _range : ranges_32)
+total_size += core_range.range.size();
clayborg wrote:
Merge this `total_size` calculation stuff into the loop above on line 836. No
need to iterate over this twice.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
clayborg wrote:
You can just assign these values with out using the `static_cast` (and yes
there are many places in this code doing these static_cast calls unecessarily:
```
descriptor.StartOfMemoryRange = addr;
descriptor.Memory.DataSize = bytes_read;
descriptor.Memory.RVA = offset_for_data;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
+}
+
+Status MinidumpFileBuilder::AddMemoryList_64(
+Process::CoreFileMemoryRanges ) {
+ Status error;
+ if (ranges.empty())
+return error;
+
+ error = AddDirectory(StreamType::Memory64List,
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast(ranges.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle64_t));
+ // Capture the starting offset for all the descriptors so we can clean them
up
+ // if needed.
+ offset_t starting_offset = GetCurrentDataEndOffset() +
sizeof(llvm::support::ulittle64_t);
+ // The base_rva needs to start after the directories, which is right after
+ // this 8 byte variable.
+ offset_t base_rva = starting_offset + (ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+ llvm::support::ulittle64_t memory_ranges_base_rva =
+ static_cast(base_rva);
+ m_data.AppendData(_ranges_base_rva,
+sizeof(llvm::support::ulittle64_t));
+
+ bool cleanup_required = false;
+ std::vector descriptors;
+ // Enumerate the ranges and create the memory descriptors so we can append
+ // them first
+ for (const auto core_range : ranges) {
+// Add the space required to store the memory descriptor
+MemoryDescriptor_64 memory_desc;
+memory_desc.StartOfMemoryRange =
+static_cast(core_range.range.start());
+memory_desc.DataSize =
+static_cast(core_range.range.size());
clayborg wrote:
remove static cast
https://github.com/llvm/llvm-project/pull/95312
___
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
+}
+
+Status MinidumpFileBuilder::AddMemoryList_64(
+Process::CoreFileMemoryRanges ) {
+ Status error;
+ if (ranges.empty())
+return error;
+
+ error = AddDirectory(StreamType::Memory64List,
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast(ranges.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle64_t));
+ // Capture the starting offset for all the descriptors so we can clean them
up
+ // if needed.
+ offset_t starting_offset = GetCurrentDataEndOffset() +
sizeof(llvm::support::ulittle64_t);
+ // The base_rva needs to start after the directories, which is right after
+ // this 8 byte variable.
+ offset_t base_rva = starting_offset + (ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+ llvm::support::ulittle64_t memory_ranges_base_rva =
+ static_cast(base_rva);
clayborg wrote:
no static cast
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -20,25 +20,104 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RangeMap.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
+ lldb_private::Target = m_process_sp->GetTarget();
+ m_expected_directories = 6;
+ // Check if OS is linux and reserve directory space for all linux specific
breakpad extension directories.
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp &&
+stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ m_saved_data_size += directory_offset;
+ Status error;
+ size_t zeroes = 0; // 8 0's
+ size_t remaining_bytes = m_saved_data_size;
+ while (remaining_bytes > 0) {
+// Keep filling in zero's until we preallocate enough space for the header
+// and directory sections.
+size_t bytes_written = std::min(remaining_bytes, sizeof(size_t));
+error = m_core_file->Write(, bytes_written);
+if (error.Fail()) {
+ error.SetErrorStringWithFormat(
+"Unable to write header and directory padding (written %zd/%zd)",
+remaining_bytes - m_saved_data_size, m_saved_data_size);
+ break;
+}
+
+remaining_bytes -= bytes_written;
+ }
clayborg wrote:
Just set the file position in the `m_core_file`:
```
m_core_file->SeekFromStart(m_saved_data_size);
```
And remove this entire while loop.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -28,17 +29,90 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
clayborg wrote:
Is there a better name for `m_saved_data_size`?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+return error;
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
clayborg wrote:
```
llvm::support::ulittle32_t memory_ranges_num(descriptors.size());
```
No need for static_cast right. Lots of places already had these incorrect extra
static_cast calls..
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +794,89 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ std::set stack_start_addresses;
+ for (const auto _range : ranges_32)
+stack_start_addresses.insert(core_range.range.start());
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto _range : ranges_32)
+total_size += core_range.range.size();
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0) {
+ // Don't double save stacks.
+ continue;
+} else if (total_size + size_to_add < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
clayborg wrote:
This can still be just:
```
total_size += size_to_add;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -81,38 +82,42 @@ Status
MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// Now offset the file by the directores so we can write them in later.
offset_t directory_offset = m_expected_directories * directory_size;
m_saved_data_size += directory_offset;
- // Replace this when we make a better way to do this.
Status error;
- Header empty_header;
- size_t bytes_written;
- bytes_written = header_size;
- error = m_core_file->Write(_header, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
-if (bytes_written != header_size)
+ size_t zeroes = 0; // 8 0's
+ size_t remaining_bytes = m_saved_data_size;
+ while (remaining_bytes > 0) {
+// Keep filling in zero's until we preallocate enough space for the header
+// and directory sections.
+size_t bytes_written = std::min(remaining_bytes, sizeof(size_t));
+error = m_core_file->Write(, bytes_written);
+if (error.Fail()) {
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
- header_size);
-return error;
- }
-
- for (uint i = 0; i < m_expected_directories; i++) {
-size_t bytes_written;
-bytes_written = directory_size;
-Directory empty_directory;
-error = m_core_file->Write(_directory, bytes_written);
-if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
-error.SetErrorStringWithFormat(
-"unable to write the directory (written %zd/%zd)", bytes_written,
-directory_size);
- return error;
+"Unable to write header and directory padding (written %zd/%zd)",
+remaining_bytes - m_saved_data_size, m_saved_data_size);
+ break;
}
+
+remaining_bytes -= bytes_written;
}
clayborg wrote:
Remove this while look and complex functionality and just call:
```
m_core_file->SeekFromStart(m_saved_data_size);
```
No need to write zeroes.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -791,26 +812,101 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+
+ std::set stack_start_addresses;
+ for (const auto _range : ranges_32)
+stack_start_addresses.insert(core_range.range.start());
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto _range : ranges_32)
+total_size += core_range.range.size();
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size += 256 + (all_core_memory_ranges.size() -
stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0)
+ // Don't double save stacks.
+ continue;
+
+if (total_size + size_to_add < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
+} else {
+ ranges_64.push_back(core_range);
+}
+ }
+
+ error = AddMemoryList_32(ranges_32);
+ if (error.Fail())
+return error;
+
+ // Add the remaining memory as a 64b range.
+ if (ranges_64.size() > 0) {
clayborg wrote:
```
if (!ranges_64.empty()) {
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_32(
+ Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ if (ranges.size() == 0)
+return error;
+
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ auto data_up = std::make_unique(GetLargestRange(ranges), 0);
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+const addr_t end = core_range.range.end();
+
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ error = AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
clayborg wrote:
indent ok here? Run clang-format on this
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +953,247 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges ) {
+ size_t max_size = 0;
+ for (const auto _range : ranges) {
+max_size = std::max(max_size, core_range.range.size());
+ }
clayborg wrote:
remove `{}` from single line for statement per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
jeffreytan81 wrote: Can this be landed safely? Do we need to update consumer side to support memory list 64? For example, if this diff is landed, and a user tries to save a minidump with memory list 64, if there is no consumer side support, will lldb crash? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -50,48 +60,75 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories();
jeffreytan81 wrote:
I would add comment to clarify that this is partially done and which fields
require later fixup.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -65,56 +66,52 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP
_sp,
if (!process_sp)
return false;
- MinidumpFileBuilder builder;
-
- Target = process_sp->GetTarget();
-
- Log *log = GetLog(LLDBLog::Object);
- error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
- if (error.Fail()) {
-LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
+ llvm::Expected maybe_core_file = FileSystem::Instance().Open(
+ outfile, File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
+ if (!maybe_core_file) {
+error = maybe_core_file.takeError();
return false;
}
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
- error = builder.AddModuleList(target);
+ Target = process_sp->GetTarget();
+ builder.AddHeaderAndCalculateDirectories();
+ Log *log = GetLog(LLDBLog::Object);
+ error = builder.AddSystemInfo();
if (error.Fail()) {
-LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString());
+LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
return false;
}
- builder.AddMiscInfo(process_sp);
+ builder.AddModuleList();
jeffreytan81 wrote:
Let's handle the failure from `AddModuleList()`. I am pretty sure I have seen a
failure once.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -65,56 +66,52 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP
_sp,
if (!process_sp)
return false;
- MinidumpFileBuilder builder;
-
- Target = process_sp->GetTarget();
-
- Log *log = GetLog(LLDBLog::Object);
- error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
- if (error.Fail()) {
-LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
+ llvm::Expected maybe_core_file = FileSystem::Instance().Open(
jeffreytan81 wrote:
Let's add a block of comment explaining the high level of the serialization
logic. Most importantly:
1. Physical layout (header first, directories next, memory list has to be the
last and why)
2. Later fixup needs to be done to header/directories after everything to the
important fields
3. Later fixup needs to be done to thread.stack field after dumping memory list
so that we can avoid duplication
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -50,48 +60,75 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
- lldb_private::Status AddSystemInfo(const llvm::Triple _triple);
+ lldb_private::Status AddSystemInfo();
// Add ModuleList stream, containing information about all loaded modules
// at the time of saving minidump.
- lldb_private::Status AddModuleList(lldb_private::Target );
+ lldb_private::Status AddModuleList();
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+ lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
+ void AddExceptions();
// Add MiscInfo stream, mainly providing ProcessId
- void AddMiscInfo(const lldb::ProcessSP _sp);
+ void AddMiscInfo();
// Add informative files about a Linux process
- void AddLinuxFileStreams(const lldb::ProcessSP _sp);
- // Dump the prepared data into file. In case of the failure data are
- // intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ void AddLinuxFileStreams();
+
+ lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
+
+ // Run cleanup and write all remaining bytes to file
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
jeffreytan81 wrote:
1. You should add comment explaining what is "FixThreads()" doing and why we
need it.
2. `FixThreads` is a very generic name, but you are really fixing up the stack
field only. So rename to "FixupThreadStack()" to be more specific.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +794,89 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ std::set stack_start_addresses;
jeffreytan81 wrote:
If you do not need ordering of the elements in the set, use
`std::unordered_set`.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -480,42 +557,32 @@ class ArchThreadContexts {
}
};
-// Function returns start and size of the memory region that contains
-// memory location pointed to by the current stack pointer.
-llvm::Expected>
-findStackHelper(const lldb::ProcessSP _sp, uint64_t rsp) {
- MemoryRegionInfo range_info;
- Status error = process_sp->GetMemoryRegionInfo(rsp, range_info);
- // Skip failed memory region requests or any regions with no permissions.
- if (error.Fail() || range_info.GetLLDBPermissions() == 0)
-return llvm::createStringError(
-std::errc::not_supported,
-"unable to load stack segment of the process");
-
- // This is a duplicate of the logic in
- // Process::SaveOffRegionsWithStackPointers but ultimately, we need to only
- // save up from the start of the stack down to the stack pointer
- const addr_t range_end = range_info.GetRange().GetRangeEnd();
- const addr_t red_zone = process_sp->GetABI()->GetRedZoneSize();
- const addr_t stack_head = rsp - red_zone;
- if (stack_head > range_info.GetRange().GetRangeEnd()) {
-range_info.GetRange().SetRangeBase(stack_head);
-range_info.GetRange().SetByteSize(range_end - stack_head);
- }
-
- const addr_t addr = range_info.GetRange().GetRangeBase();
- const addr_t size = range_info.GetRange().GetByteSize();
-
- if (size == 0)
-return llvm::createStringError(std::errc::not_supported,
- "stack segment of the process is empty");
-
- return std::make_pair(addr, size);
+Status MinidumpFileBuilder::FixThreads() {
+ Status error;
+ // If we have anything in the heap flush it.
+ FlushToDisk();
+ m_core_file->SeekFromStart(m_thread_list_start);
+ for (auto : m_thread_by_range_start) {
+// The thread objects will get a new memory descriptor added
+// When we are emitting the memory list and then we write it here
+llvm::minidump::Thread thread = pair.second;
jeffreytan81 wrote:
I do not think you need to copy:
```
const llvm::minidump::Thread& thread
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -50,48 +60,75 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
- lldb_private::Status AddSystemInfo(const llvm::Triple _triple);
+ lldb_private::Status AddSystemInfo();
// Add ModuleList stream, containing information about all loaded modules
// at the time of saving minidump.
- lldb_private::Status AddModuleList(lldb_private::Target );
+ lldb_private::Status AddModuleList();
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+ lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
+ void AddExceptions();
// Add MiscInfo stream, mainly providing ProcessId
- void AddMiscInfo(const lldb::ProcessSP _sp);
+ void AddMiscInfo();
// Add informative files about a Linux process
- void AddLinuxFileStreams(const lldb::ProcessSP _sp);
- // Dump the prepared data into file. In case of the failure data are
- // intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ void AddLinuxFileStreams();
+
+ lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
+
+ // Run cleanup and write all remaining bytes to file
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::offset_t GetCurrentDataEndOffset() const;
+ // Stores directories to fill in later
std::vector m_directories;
+ // When we write off the threads for the first time, we need to clean them up
+ // and give them the correct RVA once we write the stack memory list.
+ std::map m_thread_by_range_start;
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
+ lldb::ProcessSP m_process_sp;
+
+ uint m_expected_directories = 0;
+ uint64_t m_saved_data_size = 0;
+ lldb::offset_t m_thread_list_start = 0;
+ // We set the max write amount to 128 mb, this is arbitrary
+ // but we want to try to keep the size of m_data small
+ // and we will only exceed a 128 mb buffer if we get a memory region
+ // that is larger than 128 mb.
+ static constexpr size_t m_write_chunk_max = (1024 * 1024 * 128);
+
+ static constexpr size_t header_size = sizeof(llvm::minidump::Header);
+ static constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
jeffreytan81 wrote:
The naming convention is not consistent. For constexpr, either
`MAX_WRITE_CHUNK` or `MaxChunkSize` for readability.
Similar for `HEADER_SIZE` and `DIRECTORY_SIZE`.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
jeffreytan81 wrote:
As you are describing it, you can see this method is doing two things. I do not
see the benefit of grouping them into one confusing named function. Let's split
it into two functions:
1. FixupHeaderDirectory()
2. FlushToFile()
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -50,48 +60,75 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
- lldb_private::Status AddSystemInfo(const llvm::Triple _triple);
+ lldb_private::Status AddSystemInfo();
// Add ModuleList stream, containing information about all loaded modules
// at the time of saving minidump.
- lldb_private::Status AddModuleList(lldb_private::Target );
+ lldb_private::Status AddModuleList();
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+ lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
+ void AddExceptions();
// Add MiscInfo stream, mainly providing ProcessId
- void AddMiscInfo(const lldb::ProcessSP _sp);
+ void AddMiscInfo();
// Add informative files about a Linux process
- void AddLinuxFileStreams(const lldb::ProcessSP _sp);
- // Dump the prepared data into file. In case of the failure data are
- // intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ void AddLinuxFileStreams();
+
+ lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
+
+ // Run cleanup and write all remaining bytes to file
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb_private::Process::CoreFileMemoryRanges );
jeffreytan81 wrote:
I would simply call `AddMemoryList64`, `AddMemoryList32` and `Is64Bit`.
Let's be consistent about the member function naming convention.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
+ // Stores directories to fill in later
std::vector m_directories;
+ // When we write off the threads for the first time, we need to clean them up
+ // and give them the correct RVA once we write the stack memory list.
+ std::map m_thread_by_range_start;
clayborg wrote:
If it is an address then `lldb::addr_t` is the right choice. I must have
confused this with another location that needed to use `lldb::offset_t` for an
offset that was being stored as a `lldb::addr_t`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +923,224 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
+
+LLDB_LOGF(log,
+ "/AddMemoryList/AddMemory/ %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_start.count(addr) > 0) {
+ m_thread_by_range_start[addr].Stack = descriptor;
+}
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_64(
+const Process::CoreFileMemoryRanges ) {
+ AddDirectory(StreamType::Memory64List,
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast(ranges.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle64_t));
+ llvm::support::ulittle64_t memory_ranges_base_rva =
+ static_cast(GetCurrentDataEndOffset());
+ m_data.AppendData(_ranges_base_rva,
+sizeof(llvm::support::ulittle64_t));
+ // Capture the starting offset, so we can do cleanup later if needed.
+ uint64_t starting_offset = GetCurrentDataEndOffset();
+
+ bool cleanup_required = false;
+ std::vector descriptors;
+ // Enumerate the ranges and create the memory descriptors so we can append
+ // them first
+ for (const auto core_range : ranges) {
+// Add the space required to store the memory descriptor
+MemoryDescriptor_64 memory_desc;
+memory_desc.StartOfMemoryRange =
+static_cast(core_range.range.start());
+memory_desc.DataSize =
+static_cast(core_range.range.size());
+descriptors.push_back(memory_desc);
+// Now write this memory descriptor to the buffer.
+m_data.AppendData(_desc, sizeof(MemoryDescriptor_64));
+ }
+
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
clayborg wrote:
move this out of the for loop and have it contain `max_size` bytes just like in
the 32 bit variant of this function.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -28,17 +29,90 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
clayborg wrote:
Can we have data already in the file here? If not we should assign
`m_saved_data_size` here:
```
m_saved_data_size = header_size;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -28,17 +29,90 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
+
+ lldb_private::Target = m_process_sp->GetTarget();
+ m_expected_directories = 6;
+ // Check if OS is linux
clayborg wrote:
```
// Check if OS is linux and reserve directory space for all linux specific
breakpad extension directories.
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +937,225 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const ProcessSP _sp, const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
clayborg wrote:
re-use the one copy of `data_up` from above, just remove this line.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -50,48 +60,75 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
- lldb_private::Status AddSystemInfo(const llvm::Triple _triple);
+ lldb_private::Status AddSystemInfo();
// Add ModuleList stream, containing information about all loaded modules
// at the time of saving minidump.
- lldb_private::Status AddModuleList(lldb_private::Target );
+ lldb_private::Status AddModuleList();
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+ lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
+ void AddExceptions();
// Add MiscInfo stream, mainly providing ProcessId
- void AddMiscInfo(const lldb::ProcessSP _sp);
+ void AddMiscInfo();
// Add informative files about a Linux process
- void AddLinuxFileStreams(const lldb::ProcessSP _sp);
- // Dump the prepared data into file. In case of the failure data are
- // intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ void AddLinuxFileStreams();
+
+ lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
clayborg wrote:
I would leave this named the same as it was before as `AddMemoryList` and move
it back up to where it used to be in this source file
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +937,225 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const ProcessSP _sp, const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
+
+LLDB_LOGF(
+log,
+"AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx,
%zx)",
+region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_start.count(addr) > 0) {
+ m_thread_by_range_start[addr].Stack = descriptor;
+}
clayborg wrote:
Still need to remove these braces from this if
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +923,224 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
clayborg wrote:
Make on DataBufferHeap outside of this loop and re-use it:
```
lldb::offset_t max_size = 0;
for (const auto _range : ranges) {
if (core_range.renge.size() > max_size)
max_size = core_range.renge.size();
}
auto data_up = std::make_unique(max_size, 0);
```
auto data_up = std::make_unique(size, 0);
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +794,89 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ std::set stack_start_addresses;
+ for (const auto _range : ranges_32)
+stack_start_addresses.insert(core_range.range.start());
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto _range : ranges_32)
+total_size += core_range.range.size();
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0) {
+ // Don't double save stacks.
+ continue;
+} else if (total_size + size_to_add < UINT32_MAX) {
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
clayborg wrote:
Change these two lines to:
```
total_size += size_to_add;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +794,89 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
+return error;
+ std::set stack_start_addresses;
+ for (const auto _range : ranges_32)
+stack_start_addresses.insert(core_range.range.start());
+
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto _range : ranges_32)
+total_size += core_range.range.size();
+
+ if (total_size >= UINT32_MAX) {
+error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
+return error;
+ }
+
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
+if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional
padding
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto _range : all_core_memory_ranges) {
+addr_t size_to_add =
+core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+if (stack_start_addresses.count(core_range.range.start()) > 0) {
+ // Don't double save stacks.
+ continue;
+} else if (total_size + size_to_add < UINT32_MAX) {
clayborg wrote:
No need for `else if` if we continue:
```
if (stack_start_addresses.count(core_range.range.start()) > 0)
continue; // Stacks are already in ranges_32
if (total_size + size_to_add < UINT32_MAX)
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
clayborg wrote:
It would be nice to just have a `Finalize()` that does what `CompleteFile` and
`FinalizeFile` and `WriteAndFinish` would do?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -858,10 +937,225 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP
_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(
+const ProcessSP _sp, const Process::CoreFileMemoryRanges ) {
+ std::vector descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+// Take the offset before we write.
+const size_t offset_for_data = GetCurrentDataEndOffset();
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
+
+LLDB_LOGF(
+log,
+"AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx,
%zx)",
+region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail() || bytes_read == 0) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error:
%s",
+bytes_read, error.AsCString());
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
+} else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+size);
+}
+
+MemoryDescriptor descriptor;
+descriptor.StartOfMemoryRange =
+static_cast(addr);
+descriptor.Memory.DataSize =
+static_cast(bytes_read);
+descriptor.Memory.RVA =
+static_cast(offset_for_data);
+descriptors.push_back(descriptor);
+if (m_thread_by_range_start.count(addr) > 0) {
+ m_thread_by_range_start[addr].Stack = descriptor;
+}
+
+// Add the data to the buffer, flush as needed.
+error = AddData(data_up->GetBytes(), bytes_read);
+if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast(descriptors.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_64(
+const ProcessSP _sp, const Process::CoreFileMemoryRanges ) {
+ AddDirectory(StreamType::Memory64List,
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
+
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast(ranges.size());
+ m_data.AppendData(_ranges_num, sizeof(llvm::support::ulittle64_t));
+ llvm::support::ulittle64_t memory_ranges_base_rva =
+ static_cast(GetCurrentDataEndOffset());
+ m_data.AppendData(_ranges_base_rva,
+sizeof(llvm::support::ulittle64_t));
+ // Capture the starting offset, so we can do cleanup later if needed.
+ uint64_t starting_offset = GetCurrentDataEndOffset();
+
+ bool cleanup_required = false;
+ std::vector descriptors;
+ // Enumerate the ranges and create the memory descriptors so we can append
+ // them first
+ for (const auto core_range : ranges) {
+// Add the space required to store the memory descriptor
+MemoryDescriptor_64 memory_desc;
+memory_desc.StartOfMemoryRange =
+static_cast(core_range.range.start());
+memory_desc.DataSize =
+static_cast(core_range.range.size());
+descriptors.push_back(memory_desc);
+// Now write this memory descriptor to the buffer.
+m_data.AppendData(_desc, sizeof(MemoryDescriptor_64));
+ }
+
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ size_t region_index = 0;
+ for (const auto _range : ranges) {
+const addr_t addr = core_range.range.start();
+const addr_t size = core_range.range.size();
+auto data_up = std::make_unique(size, 0);
+
+LLDB_LOGF(log,
+ "AddMemoryList_64 %zu/%zu reading memory for region (%zu bytes) "
+ "[%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
+++region_index;
+
+const size_t bytes_read =
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+if (error.Fail()) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu,
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
clayborg wrote:
yeah, always good to minimize code changes to make reviewing easier
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
Jlalond wrote:
I would say we should error, especially with the reduced stacks since !92002 if
the stacks did exceed 4gb this would be a case where we wouldn't fully be able
to support the user.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
Jlalond wrote:
To start, this is not a good name, but we need at least two methods:
- One method to just write data from the buffer to the current end of the file
- One method to finalize and run header/directory cleanup.
When trying to get this done I couldn't come up with a good name, so I am
soliciting ideas. I think going forward some better version could be
`CompleteFile()` `FinalizeFile()` `WriteAndFinish()` but I don't like any of
them
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
+ // Stores directories to fill in later
std::vector m_directories;
+ // When we write off the threads for the first time, we need to clean them up
+ // and give them the correct RVA once we write the stack memory list.
+ std::map m_thread_by_range_start;
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
+ uint m_expected_directories = 0;
+ uint64_t m_saved_data_size = 0;
+ size_t m_thread_list_start = 0;
+ // We set the max write amount to 128 mb
+ // Linux has a signed 32b - some buffer on writes
+ // and we have guarauntee a user memory region / 'object' could be over 2gb
+ // now that we support 64b memory dumps.
+ static constexpr size_t m_write_chunk_max = (1024 * 1024 * 128);
Jlalond wrote:
Avoiding OS limitations was the goal. Originally when I was testing large
minidumps, I created a large array, that was 5.2gb in size. Linux has a max
write byte size that is slightly less than `SSIZE_T` max. However that also
ends up being platform specific, and we should probably make the write size the
architecture byte width max value for `SSIZE_T`.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
Jlalond wrote:
It's actually arbitrary. I wanted to make sure we had some byte aligned padding
before the end of the file, so that all the MemoryDescriptors64 could be 32b
addressable with the data after them.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
+ // Stores directories to fill in later
std::vector m_directories;
+ // When we write off the threads for the first time, we need to clean them up
+ // and give them the correct RVA once we write the stack memory list.
+ std::map m_thread_by_range_start;
Jlalond wrote:
In this case, the map is the start of the memory range, and the corresponding
thread. So I think addr_t is more clear. If it is not, I'll take map name
recommendations.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
Jlalond wrote:
I responded above on this, I tried to minimize allocating vectors and reduce
iterations by just removing what will fit in 32b.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
Jlalond wrote:
The reverse is so that I can remove them in place, this was so that I could use
two lists. The reason being is I already get the stacks in my first call to
process, and then I call process again with the user specific style. This is so
stacks are the first items in the memory list for the thread RVA's.
So I remove anything from the second call to process and instead move it to the
list of ranges I pass to memorylist, and the remaining go to memorylist64.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -40,7 +46,7 @@ lldb_private::Status WriteString(const std::string _write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder() = default;
+ MinidumpFileBuilder(lldb::FileUP&& core_file):
m_core_file(std::move(core_file)) {};
Jlalond wrote:
I agree with this. I also think it reduces the split responsibility with
`ObjectfileMinidump`
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP _sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP _sp,
+ const lldb_private::Process::CoreFileMemoryRanges );
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
clayborg wrote:
return `lldb::offset_t` instead of `lldb::addr_t`.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
clayborg wrote:
We know stacks need to go into the `ranges_32` list so this could be:
```
// Thread stacks must use 32 bit memory ranges as that is all that
// the thread structure allows
error = process_sp->CalculateCoreFileSaveRanges(
SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
clayborg wrote:
These 3 lines of comments seem out of place for the code that comes beneath it,
place closer to the code that enforces this
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
clayborg wrote:
These haven't changed at all, so I would move them back to this line. No need
to make changes to this header file if things haven't changed (`AddThreadList`
and `AddExceptions`). So make as much of this the same as it was before by
reverting the above 3 lines and removing them from below.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
+addr_t size_to_add =
+ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+// filter out the stacks and check if it's below 32b max.
+if (stack_ranges.count(ranges[i].range.start()) > 0) {
+ ranges.erase(ranges.begin() + i);
+} else if (total_size + size_to_add < UINT32_MAX) {
+ ranges_for_memory_list.push_back(ranges[i]);
+ total_size += ranges[i].range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
+ ranges.erase(ranges.begin() + i);
+} else {
+ break;
+}
+ }
+ error = AddMemoryList_32(process_sp, ranges_for_memory_list);
+ if (error.Fail())
+return error;
+
+ // Add the remaining memory as a 64b range.
+ if (ranges.size() > 0) {
+error = AddMemoryList_64(process_sp, ranges);
clayborg wrote:
```
if (!ranges_64.empty()) {
error = AddMemoryList_64(process_sp, ranges_64);
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
clayborg wrote:
remove `{}` from single line `for` statement and use `ranges_32` and rename
`stack_ranges` to `stack_start_addresses` since that is what we are storing:
```
std::set stack_start_addresses;
for (const auto _range : ranges_32)
stack_start_addresses.insert(core_range.range.start());
```
We also need to make sure the current `ranges_32` doesn't exceed 4GB or we must
error out since the thread stacks must be under 4GB. Or we can avoid fixing the
stack memory descriptor for any threads that exceed the 4GB barrier
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP _file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
+
clayborg wrote:
Revert these two lines and use the old ones that were removed above.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
clayborg wrote:
What is 256 here? And why do we care about `ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64))`?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP _sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP _sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP _sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP _sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP _sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP _sp);
clayborg wrote:
Revert to be the same as the lines removed from above to minimize changes to
this file
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -40,7 +46,7 @@ lldb_private::Status WriteString(const std::string _write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder() = default;
+ MinidumpFileBuilder(lldb::FileUP&& core_file):
m_core_file(std::move(core_file)) {};
clayborg wrote:
We should a `const lldb::ProcessSP _sp` to the constructor and remove
all `const lldb::ProcessSP _sp` arguemnts from the methods below. We
never want to change processes while creating a minidump. We should store the
process shared pointer as an instance variable and then the methods that used
to take the `const lldb::ProcessSP _sp` as an argument now use the
instance variable:
```
lldb::ProcessSP m_process_sp;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
clayborg wrote:
It might be useful to create two Process::CoreFileMemoryRanges lists: one for
32 bit and one for 64 bit and have a variable that keeps track of what was
already added:
```
uint64_t ranges_32_size = 0;
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
```
Then we can add regions to each list as needed below without having to remove
anything from the `ranges` list below?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
clayborg wrote:
Why are we iterating over the ranges in reverse order? It might be easier to
iterate over the ranges in the normal order and then adding the right ranges to
either the `ranges_32` or `ranges_64` list as needed? So this could be just:
```
for (const auto : all_ranges) {
lldb::offset_t range_32_size = range.range.size() +
sizeof(llvm::minidump::MemoryDescriptor);
if (stack_start_addresses.count(ranges[i].range.start()) > 0)
continue; // Stack range that was already added to range_32
if (total_size + range_32_size < UINT32_MAX) {
ranges_32.push_back(range);
total_size += range_32_size;
} else {
ranges_64.push_back(range);
}
}
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP _file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP _sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto _range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
clayborg wrote:
single line if statements should have not `{}`:
```
if (error.Fail())
return error;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff d6bbe2e20ff21cc2d31106742dfe1711ae5c641e
c592936dedb0dc494b03144d741ce57ac0b27809 --
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
llvm/include/llvm/BinaryFormat/Minidump.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 3c1d88652c..ca09e89cd6 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -24,7 +24,6 @@
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
-#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
@@ -311,10 +310,12 @@ Status MinidumpFileBuilder::AddModuleList(Target )
{
auto maybe_mod_size = getModuleFileSize(target, mod);
if (!maybe_mod_size) {
llvm::Error mod_size_err = maybe_mod_size.takeError();
- llvm::handleAllErrors(std::move(mod_size_err), [&](const
llvm::ErrorInfoBase ) {
-error.SetErrorStringWithFormat("Unable to get the size of module %s:
%s.",
- module_name.c_str(), E.message().c_str());
- });
+ llvm::handleAllErrors(std::move(mod_size_err),
+[&](const llvm::ErrorInfoBase ) {
+ error.SetErrorStringWithFormat(
+ "Unable to get the size of module %s: %s.",
+ module_name.c_str(), E.message().c_str());
+});
return error;
}
@@ -950,10 +951,10 @@ Status MinidumpFileBuilder::AddMemoryList_32(
const addr_t size = core_range.range.size();
auto data_up = std::make_unique(size, 0);
-LLDB_LOGF(
-log,
-"AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx,
%zx)",
-region_index, ranges.size(), size, addr, addr + size);
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region (%zu bytes) "
+ "[%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
++region_index;
const size_t bytes_read =
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 178318a2d6..441eef388b 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -46,7 +46,8 @@ lldb_private::Status WriteString(const std::string _write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder(lldb::FileUP&& core_file):
m_core_file(std::move(core_file)) {};
+ MinidumpFileBuilder(lldb::FileUP &_file)
+ : m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder =(const MinidumpFileBuilder &) = delete;
``
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
llvmbot wrote:
@llvm/pr-subscribers-llvm-binary-utilities
Author: Jacob Lalonde (Jlalond)
Changes
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.
---
Patch is 36.01 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/95312.diff
5 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
(+412-118)
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
(+52-15)
- (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
(+23-25)
- (modified) llvm/include/llvm/BinaryFormat/Minidump.h (+5)
- (modified) llvm/lib/ObjectYAML/MinidumpEmitter.cpp (+2)
``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7231433619ffb..3c1d88652cc92 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,25 +20,98 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(
+const lldb_private::Target , const lldb::ProcessSP _sp) {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
+ m_expected_directories = 6;
+ // Check if OS is linux
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp &&
+stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ m_saved_data_size += directory_offset;
+ // Replace this when we make a better way to do this.
+ Status error;
+ Header empty_header;
+ size_t bytes_written;
+ bytes_written = header_size;
+ error = m_core_file->Write(_header, bytes_written);
+ if (error.Fail() || bytes_written != header_size) {
+if (bytes_written != header_size)
+ error.SetErrorStringWithFormat(
+ "unable to write the header (written %zd/%zd)", bytes_written,
+ header_size);
+return error;
+ }
+
+
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/Jlalond created
https://github.com/llvm/llvm-project/pull/95312
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.
>From 810865580a4d8ea3f48e56f3964602fba6087ddf Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Wed, 12 Jun 2024 13:57:10 -0700
Subject: [PATCH 1/5] Squash minidump-64-protoype.
This is a change to reorder LLDB's minidumps to support 64b memory
lists/minidumps while still keeping all directories under the 32b limit.
Better O(N), not N^2 cleanup code
---
.../Minidump/MinidumpFileBuilder.cpp | 520 ++
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 55 +-
.../Minidump/ObjectFileMinidump.cpp | 38 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 11 +
llvm/include/llvm/Object/Minidump.h | 7 +
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 7 +
6 files changed, 482 insertions(+), 156 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7231433619ffb..c08b5206720ab 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,35 +20,106 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const
lldb_private::Target , const lldb::ProcessSP _sp) {
+ // First set the offset on the file, and on the bytes saved
+ //TODO: advance the core file.
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
(corresponding memory list for stacks)
+ // And an additional memory list for non-stacks.
+ m_expected_directories = 6;
+ // Check if OS is linux
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp && stop_info_sp->GetStopReason() ==
StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ //TODO: advance the core file.
+ m_saved_data_size += directory_offset;
+ // Replace this when we make a better way to do this.
+ Status error;
+ Header empty_header;
+ size_t bytes_written;
+ bytes_written = header_size;
+ error = m_core_file->Write(_header, bytes_written);
+ if (error.Fail() || bytes_written !=
