[Lldb-commits] [lldb] Revert "[LLDB] DebugInfoD tests: attempt to fix Fuchsia build" (PR #98101)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/98101 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[LLDB] DebugInfoD tests: attempt to fix Fuchsia build" (PR #98101)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/98101 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/96800 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: Here is a PR to fix the flaky test: https://github.com/llvm/llvm-project/pull/96800 https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
https://github.com/clayborg created
https://github.com/llvm/llvm-project/pull/96800
When we check for the "struct CustomType" in the NODWP, we can just make sure
that we have both types showing up, the next tests will validate the types are
correct. Also added a "-DAG" to the integer and float types. This should fix
the flakiness in this test.
>From a874efd2b746c1f4fcf426f7d2426d28230e7714 Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Wed, 26 Jun 2024 10:12:05 -0700
Subject: [PATCH] Fix the test to deal with non-deterministic output.
When we check for the "struct CustomType" in the NODWP, we can just make sure
that we have both types showing up, the next tests will validate the types are
correct. Also added a "-DAG" to the integer and float types. This should fix
the flakiness in this test.
---
.../DWARF/x86/dwp-foreign-type-units.cpp | 22 +--
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 8dd5a5472ed4c..4df1b33dd7d91 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -30,24 +30,14 @@
// RUN: -o "type lookup CustomType" \
// RUN: -b %t | FileCheck %s --check-prefix=NODWP
// NODWP: (lldb) type lookup IntegerType
-// NODWP-NEXT: int
-// NODWP-NEXT: unsigned int
+// NODWP-DAG: int
+// NODWP-DAG: unsigned int
// NODWP: (lldb) type lookup FloatType
-// NODWP-NEXT: double
-// NODWP-NEXT: float
+// NODWP-DAG: double
+// NODWP-DAG: float
// NODWP: (lldb) type lookup CustomType
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef int IntegerType;
-// NODWP-NEXT: typedef double FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef unsigned int IntegerType;
-// NODWP-NEXT: typedef float FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
+// NODWP: struct CustomType {
+// NODWP: struct CustomType {
// Check when we make the .dwp file with %t.main.dwo first so it will
// pick the type unit from %t.main.dwo. Verify we find only the types from
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add format eFormatEnumWithValues to ensure raw enum value is always shown (PR #90059)
clayborg wrote: Sorry for the delay, I am still wondering if we just want to have the value objects for enums return the text name as the value, and have the summary show the value as an appropriate integer if the format is eFormatEnum. When you display registers bitfields do you have a ValueObject? If so we can modify the `ValueObject::GetSummary()` to return the integer value, but only if we have the format set to `eFormatEnum`. The output for variables would be similar to what your tests expect, but we would just modify the summary string that we return and then we don't need the new format. One reason I like this way of doing things is if you call "valobj.GetValue()", and the format is set the enum with value, we will get "foo (1)" as the value back from the value object. I would rather call `valobj.GetValue()` and get `"foo"` and then call `valojb.GetSummary()` and get `"1"`, or of course you can always just call `valobj.GetValueAsUnsigned(...)` or `valobj.GetValueAsSigned(...)`. If we have special display code for the register bitfields, they can call these APIs when displaying register by default? https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/94794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
@@ -90,17 +90,9 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
if (!process_sp)
return;
- lldb::ModuleSP module_sp = GetRuntimeModuleSP();
-
Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
- module_sp, process_sp, ConstString("sanitizers_address_on_report"));
-
- if (!breakpoint) {
-breakpoint = ReportRetriever::SetupBreakpoint(
-module_sp, process_sp,
-ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
- }
-
+ GetRuntimeModuleSP(), process_sp,
+ ConstString("sanitizers_address_on_report"));
clayborg wrote:
This doesn't look right. Is this left over from the older code? Remove this?
https://github.com/llvm/llvm-project/pull/94794
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/94794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
@@ -287,7 +287,7 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec
_file_spec,
static constexpr llvm::StringLiteral k_zip_separator("!/");
size_t pos = source_file.find(k_zip_separator);
if (pos != std::string::npos)
-source_file = source_file.substr(0, pos);
+source_file = source_file.resize(0, pos);
clayborg wrote:
`std::string::resize()` returns `void` so this won't even compile. The right
fix is:
```
source_file.resize(pos);
```
https://github.com/llvm/llvm-project/pull/94785
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #96569)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/96569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix tests for FindInMemory SB API introduced in #95007. (PR #96565)
https://github.com/clayborg commented: Just add a description to what this test had wrong, what is fixed and any other needed details and this should be good to go https://github.com/llvm/llvm-project/pull/96565 ___ 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] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: An extra character snuck in and messed with the buildbots, fixed with: ``` commit fc066ca1c32b4aef549f3e371dc70589804aba0f (HEAD -> main, origin/main, origin/HEAD) Author: Greg Clayton Date: Mon Jun 24 10:15:55 2024 -0700 Fix buildbots for https://github.com/llvm/llvm-project/pull/87740 ``` https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] fc066ca - Fix buildbots for https://github.com/llvm/llvm-project/pull/87740
Author: Greg Clayton
Date: 2024-06-24T10:16:39-07:00
New Revision: fc066ca1c32b4aef549f3e371dc70589804aba0f
URL:
https://github.com/llvm/llvm-project/commit/fc066ca1c32b4aef549f3e371dc70589804aba0f
DIFF:
https://github.com/llvm/llvm-project/commit/fc066ca1c32b4aef549f3e371dc70589804aba0f.diff
LOG: Fix buildbots for https://github.com/llvm/llvm-project/pull/87740
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index bd81618ac914d..70aa4b9e1f203 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1746,7 +1746,7 @@ SymbolFileDWARF
*SymbolFileDWARF::GetDIERefSymbolFile(const DIERef _ref) {
if (file_index) {
// We have a SymbolFileDWARFDebugMap, so let it find the right file
-\if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
+if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
return debug_map->GetSymbolFileByOSOIndex(*file_index);
// Handle the .dwp file case correctly
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
https://github.com/clayborg updated
https://github.com/llvm/llvm-project/pull/87740
>From 3f99b41eac0e04e15bdb99bea2ee75703936ea00 Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/12] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed12..056c6d4b0605f8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea98..4d4555a3382529 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed4..d815d345b08ee7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
+ }
+ return result;
+}
+
llvm::DenseSet
DebugNamesDWARFIndex::GetUnits(const DebugNames _names) {
llvm::DenseSet result;
@@ -48,17 +60,22 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames
_names) {
return result;
}
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry ) const
{
+ std::optional type_sig = entry.getForeignTUTypeSignature();
+ if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/95007 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
clayborg wrote:
Add a comment about why we need to check the alignment here:
```
// We need to check the alignment because the FindInMemory uses a special
algorithm to efficiently search mememory but doesn't support alignment.
```
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
clayborg wrote:
No sense in just incrementing by 1, lets increment by an aligned value.
```
start = llvm::alignTo(start+1, alignment);
```
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorString("max_matches must be greater than zero");
+return matches;
+ }
+
+ int resolved_ranges = 0;
+ Target = GetTarget();
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_matches) {
+ break;
+}
+const AddressRange = ranges[i];
+if (range.IsValid() == false) {
+ continue;
+}
+
+const lldb::addr_t start_addr =
+range.GetBaseAddress().GetLoadAddress();
+if (start_addr == LLDB_INVALID_ADDRESS) {
+ continue;
+}
+++resolved_ranges;
+const lldb::addr_t end_addr = start_addr + range.GetByteSize();
+DoFindInMemory(start_addr, end_addr, buf, size, matches, alignment,
+ max_matches);
+ }
+
+ if (resolved_ranges > 0) {
+error.Clear();
+ } else {
+error.SetErrorString("unable to resolve any ranges");
+ }
clayborg wrote:
remove single if/else statement '{}' per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorString("max_matches must be greater than zero");
+return matches;
+ }
+
+ int resolved_ranges = 0;
+ Target = GetTarget();
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_matches) {
+ break;
+}
+const AddressRange = ranges[i];
+if (range.IsValid() == false) {
+ continue;
+}
+
+const lldb::addr_t start_addr =
+range.GetBaseAddress().GetLoadAddress();
+if (start_addr == LLDB_INVALID_ADDRESS) {
+ continue;
+}
clayborg wrote:
remove single if statement '{}' per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorString("max_matches must be greater than zero");
+return matches;
+ }
+
+ int resolved_ranges = 0;
+ Target = GetTarget();
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_matches) {
+ break;
+}
+const AddressRange = ranges[i];
+if (range.IsValid() == false) {
+ continue;
+}
clayborg wrote:
remove single if statement '{}' per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
clayborg wrote:
All these lines can be:
```
lldb::addr_t start = llvm::alignTo(start_addr, alignment);
```
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorString("max_matches must be greater than zero");
+return matches;
+ }
+
+ int resolved_ranges = 0;
+ Target = GetTarget();
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_matches) {
+ break;
+}
clayborg wrote:
remove single if statement '{}' per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: All issues resolved except the change of `GetDIERefSymbolFile`. Let me know if you agree with my comments, or want a `GetSymbolFileByFileIndex(std::optional file_idx)`. https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -1727,45 +1727,61 @@ lldb::ModuleSP
SymbolFileDWARF::GetExternalModule(ConstString name) {
return pos->second;
}
-DWARFDIE
-SymbolFileDWARF::GetDIE(const DIERef _ref) {
- // This method can be called without going through the symbol vendor so we
- // need to lock the module.
- std::lock_guard guard(GetModuleMutex());
-
- SymbolFileDWARF *symbol_file = nullptr;
-
+SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef _ref) {
// Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
// must make sure we use the correct DWARF file when resolving things. On
// MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
// SymbolFileDWARF classes, one for each .o file. We can often end up with
// references to other DWARF objects and we must be ready to receive a
// "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
// instance.
+
std::optional file_index = die_ref.file_index();
+
+ // If the file index matches, then we have the right SymbolFileDWARF already.
+ // This will work for both .dwo file and DWARF in .o files for mac. Also if
+ // both the file indexes are invalid, then we have a match.
+ if (GetFileIndex() == file_index)
+return this;
+
+ // If we are currently in a .dwo file and our file index doesn't match we
need
+ // to let the base symbol file handle this.
+ SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null(this);
+ if (dwo)
+return dwo->GetDIERefSymbolFile(die_ref);
+
if (file_index) {
-if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
- symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO
case
- if (symbol_file)
-return symbol_file->DebugInfo().GetDIE(die_ref.section(),
- die_ref.die_offset());
- return DWARFDIE();
+SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
+if (debug_map) {
+ // We have a SymbolFileDWARFDebugMap, so let it find the right file
+ return debug_map->GetSymbolFileByOSOIndex(*file_index);
+} else {
+ // Handle the .dwp file case correctly
+ if (*file_index == DIERef::k_file_index_mask)
+return GetDwpSymbolFile().get(); // DWP case
+
+ // Handle the .dwo file case correctly
+ return DebugInfo()
+ .GetUnitAtIndex(*die_ref.file_index())
+ ->GetDwoSymbolFile(); // DWO case
}
+ }
+ return this;
+}
-if (*file_index == DIERef::k_file_index_mask)
- symbol_file = GetDwpSymbolFile().get(); // DWP case
-else
- symbol_file = this->DebugInfo()
-.GetUnitAtIndex(*die_ref.file_index())
-->GetDwoSymbolFile(); // DWO case
- } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
+DWARFDIE
+SymbolFileDWARF::GetDIE(const DIERef _ref) {
+ if (die_ref.die_offset() == DW_INVALID_OFFSET)
return DWARFDIE();
- }
+ // This method can be called without going through the symbol vendor so we
+ // need to lock the module.
+ std::lock_guard guard(GetModuleMutex());
+ SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
clayborg wrote:
It only uses the file index component, but that component might not be set in
the `DIERef` object and if it isn't, it returns the current SymbolFileDWARF
object. We can make a function like:
```
SymbolFileDWARF *GetSymbolFileByFileIndex(std::optional file_index);
```
The file index only makes sense if it comes from a DIERef object as it
understands the value and knows the magic value for the .dwp file, so the above
API is much less clear as who else knows what a file index is and would
actaully know to ask for the symbol file using such an index? Seems less clear
to me than the existing API. Let me know if feel we should still change this
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
https://github.com/clayborg updated
https://github.com/llvm/llvm-project/pull/87740
>From badd915257bb192add91696e0b8530c057bd385f Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/12] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed1..056c6d4b0605f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea9..4d4555a338252 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed..d815d345b08ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
+ }
+ return result;
+}
+
llvm::DenseSet
DebugNamesDWARFIndex::GetUnits(const DebugNames _names) {
llvm::DenseSet result;
@@ -48,17 +60,22 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames
_names) {
return result;
}
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry ) const
{
+ std::optional type_sig = entry.getForeignTUTypeSignature();
+ if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+ return
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/96243 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
clayborg wrote: I got the original PR back on track... https://github.com/llvm/llvm-project/pull/96243 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: @ayermolo: you suggestion worked. thanks. This PR is back on track... @labath: I implemented your suggestions, let me know if this look ok now https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
https://github.com/clayborg updated
https://github.com/llvm/llvm-project/pull/87740
>From badd915257bb192add91696e0b8530c057bd385f Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/11] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed1..056c6d4b0605f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea9..4d4555a338252 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed..d815d345b08ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
+ }
+ return result;
+}
+
llvm::DenseSet
DebugNamesDWARFIndex::GetUnits(const DebugNames _names) {
llvm::DenseSet result;
@@ -48,17 +60,22 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames
_names) {
return result;
}
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry ) const
{
+ std::optional type_sig = entry.getForeignTUTypeSignature();
+ if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+ return
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
https://github.com/clayborg created
https://github.com/llvm/llvm-project/pull/96243
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
- support finding type unit entries in .dwp and .dwo files
This PR is a redo of https://github.com/llvm/llvm-project/pull/87740 after that
branch had merging issues
>From c800f88b19d6e470e73c350ea5bcc88ce299f0eb Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/10] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed1..056c6d4b0605f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea9..4d4555a338252 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed..d815d345b08ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -101,3 +101,5 @@ bool SBAddressRange::GetDescription(SBStream ,
m_opaque_up->GetDescription(, target.GetSP().get());
return true;
}
+
+lldb_private::AddressRange ::ref() const { return *m_opaque_up;
}
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2800,6 +2809,10 @@ void PruneThreadPlans(); virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status ) = 0; + void DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr, + const uint8_t *buf, size_t size, AddressRanges , + size_t alignment, size_t max_matches); + clayborg wrote: We might want this to be virtual to allow core files to do this quicker than having to read all memory into a buffer. https://github.com/llvm/llvm-project/pull/95007 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -92,3 +92,7 @@ bool SBAddressRangeList::GetDescription(SBStream ,
stream << "]";
return true;
}
+
+lldb_private::AddressRanges ::ref() const {
+ return m_opaque_up->ref();
+}
clayborg wrote:
we might want to add an `assert()` call here like you did for
`SBAddressRange::ref()`?
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -64,7 +64,7 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ m_opaque_up->Clear();
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorStringWithFormat("max_matches must be greater than zero");
clayborg wrote:
use `error.SetErrorString()` if there are no formats being applied
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -58,6 +58,8 @@ class LLDB_API SBAddressRange {
friend class SBFunction;
friend class SBProcess;
+ lldb_private::AddressRange () const;
+
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
clayborg wrote:
> This class behaves quite differently from other SB API classes. Normally, the
> opaque pointer can be cleared to release the potentially more resource heavy
> private counterpart. `AddressRange` is a pretty simple class, so I understand
> that it makes things easier if we guarantee the pointer is always valid, but
> it is somewhat of a surprise.
>
> Personally, I think consistency beats the small advantage of not having to
> check the pointer. If we want to stick to this approach, I'd like to see an
> assert that makes it clear that in this class, we have a precondition that
> the pointer is always valid:
>
> ```
> assert(m_opaque_up && "opaque pointer must always be valid");
> ```
For simple classes, there is no need to clear the unique pointer, so I like
this approach for small classes.
We can use the assert in a new protected `ref()` method:
```
lldb_private::AddressRange & SBAddressRange::ref() {
assert(m_opaque_up && "opaque pointer must always be valid");
return *m_opaque_up;
}
```
And then have everything that accesses `m_opaque_up` use the `ref()` function.
It is similar to other classes and makes the code nicer when we don't see
direct uses of `m_opaque_up`
https://github.com/llvm/llvm-project/pull/95997
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/95075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/95075 ___ 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);
+
+ 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] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -29,6 +29,9 @@ struct OptionArgParser {
static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
+ static bool ToBoolean(llvm::StringRef option_name, llvm::StringRef
option_arg,
+bool fail_value, Status );
+
clayborg wrote:
If we are going to add a function here I would suggest we add it with
`llvm::Expected`:
```
llvm::Expected ToBoolean(llvm::StringRef option_arg);
```
This will return a `bool` or a `llvm::Error`. `lldb_private::Status` has a
constructor that takes an `llvm::Error`. I will show how to use this in the new
code below.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -1425,8 +1425,21 @@ let Command = "statistics dump" in {
Desc<"Dump the total possible debug info statistics. "
"Force loading all the debug information if not yet loaded, and collect "
"statistics with those.">;
+ def statistics_dump_targets: Option<"targets", "r">, Group<1>,
+Arg<"Boolean">,
+Desc<"Dump statistics for the targets, including breakpoints, expression "
+"evaluations, frame variables, etc. "
+"If both the '--targets' and the '--modules' options are 'true', a list "
+"of module identifiers will be added to the 'targets' section.">;
clayborg wrote:
Talk about the default value for this as mentioned below.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -76,13 +77,22 @@ class CommandObjectStatsDump : public CommandObjectParsed {
m_all_targets = true;
break;
case 's':
-m_stats_options.summary_only = true;
+m_stats_options.SetSummaryOnly(true);
break;
case 'f':
-m_stats_options.load_all_debug_info = true;
+m_stats_options.SetLoadAllDebugInfo(true);
+break;
+ case 'r':
+m_stats_options.SetIncludeTargets(OptionArgParser::ToBoolean(
+"--targets", option_arg, true /* doesn't matter */, error));
+break;
+ case 'm':
+m_stats_options.SetIncludeModules(OptionArgParser::ToBoolean(
+"--modules", option_arg, true /* doesn't matter */, error));
break;
case 't':
-m_stats_options.include_transcript = true;
+m_stats_options.SetIncludeTranscript(OptionArgParser::ToBoolean(
+"--transcript", option_arg, true /* doesn't matter */, error));
clayborg wrote:
Ditto.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -1425,8 +1425,21 @@ let Command = "statistics dump" in {
Desc<"Dump the total possible debug info statistics. "
"Force loading all the debug information if not yet loaded, and collect "
"statistics with those.">;
+ def statistics_dump_targets: Option<"targets", "r">, Group<1>,
+Arg<"Boolean">,
+Desc<"Dump statistics for the targets, including breakpoints, expression "
+"evaluations, frame variables, etc. "
+"If both the '--targets' and the '--modules' options are 'true', a list "
+"of module identifiers will be added to the 'targets' section.">;
+ def statistics_dump_modules: Option<"modules", "m">, Group<1>,
+Arg<"Boolean">,
+Desc<"Dump statistics for the modules, including time and size of various "
+"aspects of the module and debug information, type system, path, etc. "
+"If both the '--targets' and the '--modules' options are 'true', a list "
+"of module identifiers will be added to the 'targets' section.">;
clayborg wrote:
Talk about the default value for this and what will turn this on or off. This
defaults to `true` unless the `--summary` mode is enabled which will turn it
off unless this is specified.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -76,13 +77,22 @@ class CommandObjectStatsDump : public CommandObjectParsed {
m_all_targets = true;
break;
case 's':
-m_stats_options.summary_only = true;
+m_stats_options.SetSummaryOnly(true);
break;
case 'f':
-m_stats_options.load_all_debug_info = true;
+m_stats_options.SetLoadAllDebugInfo(true);
+break;
+ case 'r':
+m_stats_options.SetIncludeTargets(OptionArgParser::ToBoolean(
+"--targets", option_arg, true /* doesn't matter */, error));
+break;
+ case 'm':
+m_stats_options.SetIncludeModules(OptionArgParser::ToBoolean(
+"--modules", option_arg, true /* doesn't matter */, error));
clayborg wrote:
Ditto, use `llvm::Expected ToBoolean(llvm::StringRef option_arg);` as
above
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -76,13 +77,22 @@ class CommandObjectStatsDump : public CommandObjectParsed {
m_all_targets = true;
break;
case 's':
-m_stats_options.summary_only = true;
+m_stats_options.SetSummaryOnly(true);
break;
case 'f':
-m_stats_options.load_all_debug_info = true;
+m_stats_options.SetLoadAllDebugInfo(true);
+break;
+ case 'r':
+m_stats_options.SetIncludeTargets(OptionArgParser::ToBoolean(
+"--targets", option_arg, true /* doesn't matter */, error));
clayborg wrote:
Lets use the new `llvm::Expected ToBoolean(llvm::StringRef option_arg);`
function:
```
if (auto bool_or_error = OptionArgParser::ToBoolean(option_arg))
m_stats_options.SetIncludeTargets(*bool_or_error);
else
error.SetErrorStringWithFormat("failed to parse --targets boolean value: %s",
llvm::toString(bool_or_error.takeError()).c_str());
```
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add/change options in `statistics dump` to control what sections are dumped (PR #95075)
@@ -25,6 +25,15 @@ class LLDB_API SBStatisticsOptions {
void SetSummaryOnly(bool b);
bool GetSummaryOnly();
+ void SetIncludeTargets(bool b);
+ bool GetIncludeTargets() const;
+
+ void SetIncludeModules(bool b);
+ bool GetIncludeModules() const;
+
+ void SetIncludeTranscript(bool b);
+ bool GetIncludeTranscript() const;
+
clayborg wrote:
Lets add some header doc here that explains what the default values are for
each scenario so people know what these will do.
https://github.com/llvm/llvm-project/pull/95075
___
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] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: > > But we won't want a CU index back if we have a `DW_IDX_type_unit` that is a > > local type unit. Makes no sense to return a CUOffset from such entries. > > Can you elaborate on that? I believe it doesn't make sense to you, but I just > don't see the logic behind it. In particular I don't understand why it makes > sense to return the CU offset when the DW_IDX_compile_unit attribute is > explicitly specified (cases 2 and 3), but not when the compile unit is > referred to implicitly via the lack of the attribute on a single-cu index > (case 1) When we have an entry like: ``` DW_IDX_type_unit(0) DW_IDX_die_offset(0x32) ``` And type unit at index 0 is a local type unit, then the entry describes a type unit DIE in the .debug_info of the current executable. The current API we have is in `DWARFAcceleratorTable.h` for `DWARFDebugNames::Entry` is: ``` std::optional getCUOffset() const override; std::optional getLocalTUOffset() const override; std::optional getForeignTUTypeSignature() const override; ``` So if we have an entry mentioned above where the type unit lives in the local .debug_info section, we should get an invalid value back from a call to `getCUOffset()` if this is a local type unit. And we should get a valid value back from `getLocalTUOffset()`. But it makes no sense to return the first compile unit for a call to `getCUOffset()` for an entry that represents a local type unit because it isn't related to the local type unit. Lets say we do allow an entry with a local type unit to return the first CU from the .debug_names only because there is only one CU in the .debug_names table. Soon `lld` will generate a single .debug_names table with multiple CUs. We will get no CU offset back for an entry that contains a local type unit + die offset (no CU specified) when using a combined .debug_names table because it can't rely on there only being one CU in the list, but we will get a valid value back for individual .debug_names tables, which makes the API usage inconsistent between individual tables and combined tables. Any foreign type units that must be associated with a skeleton compile unit, should have both a `DW_IDX_type_unit` and a `DW_IDX_comp_unit` to make sure we can make this connection between a TU and originating CU. https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote:
> My previous message reflected my confusion about the usefulness about the
> DW_IDX_cu+DW_IDX_tu combo. Let me try to start over.
>
> The thing I don't like about this patch is the duplication between
> `getForeignTUSkeletonCUOffset` (the new function) and `getCUOffset` (the
> existing function). Since the duplication is motivated by undesirable
> behavior (result) of the existing function in some cases, this got me
> thinking if there's a way to define a single function in a way that will be
> usable in all scenarios (and still have a consistent easy-to-understand
> definition).
>
> To see if that's possible, let's consider the possible cases (I'm putting
> cases with DW_IDX_tu first, as those are the interesting ones here):
>
> 1. A single-CU index, DW_IDX_cu not present, DW_IDX_tu present
> 2. A multi-CU index, DW_IDX_cu and DW_IDX_tu present
>
> These two definitely are the most common (of those including type units)
> ones, but we can also have:
>
> 3. A single-CU index, DW_IDX_cu and DW_IDX_tu present
> 4. A multi-CU index, DW_IDX_cu not present, DW_IDX_tu present
>
> For completeness, we can also have the cases without type units. I will
> include only include one here, as I think the others are obvious:
>
> 5. A single-CU index, DW_IDX_cu and DW_IDX_tu **not** present
>
> Now that we have that, what are the possible `getCUOffset`? I can think of
> three:
>
> a) "return the value of DW_IDX_cu. Period." If an explicit value is not
> present return nothing. I don't think anyone wants this, but I'm mentioning
> it for completeness. With this definition, the function should return the cu
> offset only in the second and third cases (`NYYNN`)
> b) "return the DW_IDX_cu value, or the single CU from the index." Let's call
> this the "raw" version. With this definition, I think this function should
> return the cu offset in all cases except the fourth (that one is impossible
> to resolve): `YYYNY`
> c) "return the DW_IDX_cu value, only if the entry describes a DIE within that
> CU." Let's call this the "semantic" version. I believe a function with this
> definition should not return a CU offset only in the last case: `Y`
>
> Now, the problem I see is that the current implementation of `getCUOffset` is
> **neither** of these options. If an entry has an (explicit) DW_IDX_cu, it
> will always return it (even if it also contains DW_IDX_tu). However, an
> implicit CU will only be returned if DW_IDX_tu is not present. I.e, this
> function does a `NYNNY`.
>
> Therefore, my proposal is to redefine the function to do (b), which
> essentially amounts to deleting these two lines:
>
> ```
> if (lookup(dwarf::DW_IDX_type_unit).has_value())
> return std::nullopt;
> ```
But we won't want a CU index back if we have a `DW_IDX_type_unit` that is a
local type unit. Makes no sense to return a CUOffset from such entries. So
maybe we modify this be:
```
std::optional DWARFDebugNames::Entry::getCUIndex() const {
if (std::optional Off = lookup(dwarf::DW_IDX_compile_unit))
return Off->getAsUnsignedConstant();
// In a per-CU index, the entries without a DW_IDX_compile_unit attribute
// implicitly refer to the single CU, but only if we don't have a
// DW_IDX_type_unit that is a foreign type unit.
if (IsForeignTypeUnit())
return std::nullopt;
if (NameIdx->getCUCount() == 1)
return 0;
return std::nullopt;
}
```
Then we implement a `bool IsForeignTypeUnit() const` that just checks if there
is a DW_IDX_type_unit whose index is a foreign type unit.
>
> Besides providing a consistent definition of the function (YMMV, if you can
> provide an explanation for the current behavior, I'd like to hear it), it
> would also enable you to reuse it in `getForeignTUSkeletonCUOffset`, as that
> is exactly the behavior you need there (although, at that point, I don't
> think `getForeignTUSkeletonCUOffset` needs to exist as a separate function,
> as it will be simple enough to inline into lldb).
That is fine with me as long as you agree with the above fix.
Let me know if the above fix to `DWARFDebugNames::Entry::getCUIndex()` makes
sense.
https://github.com/llvm/llvm-project/pull/87740
___
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] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: > Most of the patch is very clean, but I'm bothered by the > `getForeignTUSkeletonCUOffset` function, how it opens up with a mostly > redundant (the callers checks this already) call to > `getForeignTUTypeSignature`, and then proceeds with a reimplementation of > `getCUOffset` (sans the type unit check). I think we could design a better > set of APIs for this, but I'm not sure what those is, because I'm unclear of > the meaning of various combinations of DW_IDX unit entries. If we have an entry with a `DW_IDX_type_unit`, it can be a normal type unit, or a foreign type unit, that depends on the indexes value. If it is less than the number of local type units, then it is a local type unit and there should be no `DW_IDX_comp_unit` in the entry, else it is a foreign type unit from a .dwo or .dwp file. If we have a .dwp file, we need the `DW_IDX_comp_unit` to be able to tell if the foreign type unit made it into the .dwp file, or if this is an entry that represents the a different type unit and we should ignore it. If we have no .dwp file, then all entries for foreign type units are valid and we don't need to check the compile unit it came from. So if you just ask an entry for its `DW_IDX_comp_unit`, you will get an answer for an entry from a normal compile unit, or for a foreign type unit with a compile unit that will be used to discriminate for the .dwp file case. So this function is named `getForeignTUSkeletonCUOffset()` to help say "this might have compile unit entry, but we only want it if this is a foreign type unit. I took me a while to understand the spec when it came to foreign type units... > What the new function (I think) essentially does is "give me the CU > associated with this entry even if the entry does not describe a DIE in this > CU" (because `DW_IDX_die_offset` is relative to a type unit)". I think this > API would make sense, even without needing to talk about type units. However, > is it actually implementable? For single-CU indexes, that's fine, because we > can kind of assume all entries are related to that CU. But what about > multi-CU indexes? The only way to determine the CU there is to have an > explicit attribute. Are we saying that each entry in a multi-CU index must > have a DW_IDX_compile_unit (in addition to a DW_IDX_type_unit)? Yes, but only if the `DW_IDX_type_unit` represents a foreign type unit, and we will only need the `DW_IDX_comp_unit` if we have a .dwp file. But since we don't know if the user will use a .dwp file, we always need to put the data for the originating compile uint in as a `DW_IDX_comp_unit`. I would rather have had a `DW_IDX_skeleton_unit` to differentiate between a `DW_IDX_comp_unit` and requiring the user to know that they can't just find the compile unit, but they must find the non-skeleton compile unit, before adding the `DW_IDX_die_offset`. But we don't have that in the spec... > > If the answer is yes, then this function can be implemented, but then I think > the current implementation of `getCUOffset` (which I interpret as "give me > the CU offset **IF** this entry is relative to that CU") doesn't make sense > -- because it treats entries with an explicit `DW_IDX_compile_unit` different > from an implicit/missing `DW_IDX_compile_unit`. It doesn't treat them differently. if you ask an entry for its `DW_IDX_compile_unit` and it doesn't have one, we can fall back to grabbing a CU from the CU table, but only if the CU table has only 1 entry in it. > And in this world, we assume that `DW_IDX_type_unit` takes precedence over > `DW_IDX_compile_unit` -- if both are present then `DW_IDX_die_offset` is > relative to the former. And I'm not sure if we would actually want to take up > space by putting both values into the index. Looking at existing producers > would be interesting, but I'm not sure if there are any (lld does not merge > type unit indexes right now, possibly for this very reason). Maybe one could > produce something with LTO? LTO and bolt and soon lld will produce these single tables with multiple compile units. And we MUST have the `DW_IDX_comp_unit` for foreign type units because type units can generate differing content and one copy of the type unit will make it into the .dwp and we need to know from which .dwo file it came and only use the entries from the matching .dwo file because the offsets can be meaningless for type units with the same signature but that came from different .dwo files. > > OTOH, if the answer is no, then the function isn't implementable in the > generic case. That doesn't mean you can't implement this feature -- which is > useful for guarding against ODR violations (exacerbated by llvm's > nonconforming type signature computation algorithm...). However, I think it > could be implemented in a much simpler way. For example, lldb could just > check if it's looking at a single-CU index, and get the CU offset that way. > No extra
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/95075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
@@ -133,7 +133,9 @@ struct ConstStringStats {
struct StatisticsOptions {
bool summary_only = false;
bool load_all_debug_info = false;
- bool include_transcript = false;
+ bool include_targets = true;
+ bool include_modules = true;
clayborg wrote:
Do we want to make all of the StatisticsOptions bool values
`std::optional` values? Then we will know if the user has set them and we
can do the reasoning. Accessors can be added to this `StatisticsOptions` class
to do the right thing. Then instead of people directly accessing each
`std::optional` we would have accessors. Something like:
```
class StatisticsOptions {
private: // Stop direct access to the member variables
std::optional m_summary_only;
std::optional m_load_all_debug_info;
std::optional m_include_transcript;
std::optional m_include_targets;
std::optional m_include_modules;
public:
// Summary only defaults to false.
bool GetSummaryOnly() const { return m_summary_only.value_or(false); }
void SetSummaryOnly(bool value) { m_summary_only = value; }
// Now we can reason about what values to return depending on m_summary_only:
bool GetIncludeTargets() const {
if (m_include_targets.has_value())
return m_include_targets.value();
// m_include_targets has no value set, so return a value base on
m_summary_only
return !GetSummaryOnly();
}
```
Then this allows you to always do the right thing depending on what options
people select.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
@@ -133,7 +133,9 @@ struct ConstStringStats {
struct StatisticsOptions {
bool summary_only = false;
bool load_all_debug_info = false;
- bool include_transcript = false;
clayborg wrote:
I would move this line that was removed back to where it was on line 136 (it is
now 138). The diff will be smaller
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
@@ -77,12 +78,33 @@ class CommandObjectStatsDump : public CommandObjectParsed {
break;
case 's':
m_stats_options.summary_only = true;
+// In the "summary" mode, the following sections should be omitted by
+// default unless their corresponding options are explicitly given.
+// If such options were processed before 's', `m_seen_options` should
+// contain them, and we will skip setting them to `false` here. If such
+// options are not yet processed, we will set them to `false` here, and
+// they will be overwritten when the options are processed.
+if (m_seen_options.find((int)'r') == m_seen_options.end())
+ m_stats_options.include_targets = false;
+if (m_seen_options.find((int)'m') == m_seen_options.end())
+ m_stats_options.include_modules = false;
+if (m_seen_options.find((int)'t') == m_seen_options.end())
+ m_stats_options.include_transcript = false;
break;
clayborg wrote:
We should put these smarts into the `StatisticsOptions` class so that we don't
have to do this logic here _and_ this means that when people use the
`SBStatisticsOptions` class via the API, we will do the right thing when using
the API and it will match the behavior here.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/95075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
@@ -44,6 +44,30 @@ void SBStatisticsOptions::SetSummaryOnly(bool b) {
bool SBStatisticsOptions::GetSummaryOnly() { return m_opaque_up->summary_only;
}
+void SBStatisticsOptions::SetIncludeTargets(bool b) {
+ m_opaque_up->include_targets = b;
+}
+
+bool SBStatisticsOptions::GetIncludeTargets() const {
+ return m_opaque_up->include_targets;
+}
+
+void SBStatisticsOptions::SetIncludeModules(bool b) {
+ m_opaque_up->include_modules = b;
+}
+
+bool SBStatisticsOptions::GetIncludeModules() const {
+ return m_opaque_up->include_modules;
+}
+
+void SBStatisticsOptions::SetIncludeTranscript(bool b) {
+ m_opaque_up->include_transcript = b;
+}
+
+bool SBStatisticsOptions::GetIncludeTranscript() const {
+ return m_opaque_up->include_transcript;
+}
+
clayborg wrote:
If we switch the internal `StatisticsOptions` over to use optionals + accessors
as suggested above, this will accurately track when and if any settings are set
here via the API, and do the right thing, just like the command line command.
https://github.com/llvm/llvm-project/pull/95075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: I think I have addressed all issues. Does anyone have time to review? https://github.com/llvm/llvm-project/pull/87740 ___ 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
