https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/138169
>From ef04502d17c36044cd5fb96f333c328c8215f354 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 1 May 2025 10:11:10 -0700
Subject: [PATCH 1/5] Add new API to expose the expected size in bytes of a
core before generation
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 13 ++++++++++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 ++
lldb/source/API/SBSaveCoreOptions.cpp | 5 +++++
lldb/source/Symbol/SaveCoreOptions.cpp | 18 ++++++++++++++++
.../TestSBSaveCoreOptions.py | 21 +++++++++++++++++++
.../sbsavecoreoptions/basic_minidump.yaml | 5 +++++
6 files changed, 64 insertions(+)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index c6d2ab6099b3c..4c051353a714e 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -119,6 +119,19 @@ class LLDB_API SBSaveCoreOptions {
/// an empty collection will be returned.
SBThreadCollection GetThreadsToSave() const;
+ /// Get the current total number of bytes the core is expected to be but not
+ /// including the overhead of the core file format. Requires a Process and
+ /// Style to be specified.
+ ///
+ /// \note
+ /// This can cause some modification of the underlying data store
+ /// as regions with no permissions, or invalid permissions will be removed
+ /// and stacks will be minified up to their stack pointer + the redzone.
+ ///
+ /// \returns
+ /// The expected size of the data contained in the core in bytes.
+ uint64_t GetCurrentSizeInBytes(SBError &error);
+
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index bcf0087fbea5c..319d44a6b0c87 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -49,6 +49,8 @@ class SaveCoreOptions {
lldb_private::ThreadCollection::collection GetThreadsToSave() const;
+ uint64_t GetCurrentSizeInBytes(Status &error);
+
void Clear();
private:
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 35b9da569dfa1..b67df513fe91b 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -114,6 +114,11 @@ void SBSaveCoreOptions::Clear() {
m_opaque_up->Clear();
}
+uint64_t SBSaveCoreOptions::GetCurrentSizeInBytes(SBError &error) {
+ LLDB_INSTRUMENT_VA(this, error);
+ return m_opaque_up->GetCurrentSizeInBytes(error.ref());
+}
+
lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
return *m_opaque_up.get();
}
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index c9f6efeb25d22..1da3e1cc9f834 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -145,6 +145,24 @@ SaveCoreOptions::GetThreadsToSave() const {
return thread_collection;
}
+uint64_t SaveCoreOptions::GetCurrentSizeInBytes(Status &error) {
+ if (!m_process_sp) {
+ error = Status::FromErrorString("Requires a process to be set.");
+ return 0;
+ }
+
+ CoreFileMemoryRanges ranges;
+ error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
+ if (error.Fail())
+ return 0;
+
+ uint64_t total_in_bytes = 0;
+ for (auto& core_range : ranges)
+ total_in_bytes += core_range.data.range.size();
+
+ return total_in_bytes;
+}
+
void SaveCoreOptions::ClearProcessSpecificData() {
// Deliberately not following the formatter style here to indicate that
// this method will be expanded in the future.
diff --git
a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index ace84e8497a59..215f8440cc68a 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -104,3 +104,24 @@ def test_removing_and_adding_insertion_order(self):
thread_collection = options.GetThreadsToSave()
self.assertEqual(thread_collection.GetSize(), 3)
self.assertIn(middle_thread, thread_collection)
+
+ def test_get_total_in_bytes(self):
+ """
+ Tests that get total in bytes properly returns an error without a
process,
+ and the readable regions with a process.
+ """
+
+ options = lldb.SBSaveCoreOptions()
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ process = self.get_basic_process()
+ memory_range = lldb.SBMemoryRegionInfo()
+ process.GetMemoryRegionInfo(0x7FFF12A84030, memory_range)
+ options.AddMemoryRegionToSave(memory_range)
+ error = lldb.SBError()
+ total = options.GetCurrentSizeInBytes(error)
+ self.assertTrue(error.Fail(), error.GetCString())
+ options.SetProcess(process)
+ total = options.GetCurrentSizeInBytes(error)
+ self.assertTrue(error.Success(), error.GetCString())
+ expected_size = memory_range.GetRegionEnd() -
memory_range.GetRegionBase()
+ self.assertEqual(total, expected_size)
diff --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
index 96302fbfb6b5c..5033787019d7b 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
+++ b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
@@ -34,3 +34,8 @@ Streams:
Stack:
Start of Memory Range: 0x00007FFFC8DFF000
Content: 'BAADBEEF'
+ - Type: Memory64List
+ Memory Ranges:
+ - Start of Memory Range: 0x7FFF12A84030
+ Data Size: 0x2FD0
+ Content : ''
>From 6f41d701d87dbcd1a09debc490ee954bfc4049c2 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 1 May 2025 10:17:00 -0700
Subject: [PATCH 2/5] Add check to EnsureValidConfiguration to filter out some
corner cases
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 6 +++---
lldb/source/Symbol/SaveCoreOptions.cpp | 6 +++++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 4c051353a714e..2066ab3e8246c 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -120,11 +120,11 @@ class LLDB_API SBSaveCoreOptions {
SBThreadCollection GetThreadsToSave() const;
/// Get the current total number of bytes the core is expected to be but not
- /// including the overhead of the core file format. Requires a Process and
+ /// including the overhead of the core file format. Requires a Process and
/// Style to be specified.
- ///
+ ///
/// \note
- /// This can cause some modification of the underlying data store
+ /// This can cause some modification of the underlying data store
/// as regions with no permissions, or invalid permissions will be removed
/// and stacks will be minified up to their stack pointer + the redzone.
///
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index 1da3e1cc9f834..cf6fc99f7236e 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -151,13 +151,17 @@ uint64_t SaveCoreOptions::GetCurrentSizeInBytes(Status
&error) {
return 0;
}
+ error = EnsureValidConfiguration(m_process_sp);
+ if (error.Fail())
+ return 0;
+
CoreFileMemoryRanges ranges;
error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
if (error.Fail())
return 0;
uint64_t total_in_bytes = 0;
- for (auto& core_range : ranges)
+ for (auto &core_range : ranges)
total_in_bytes += core_range.data.range.size();
return total_in_bytes;
>From 53d8c0b4ec3f55db71f5d1aa3c56ebbaa860f807 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 1 May 2025 10:20:51 -0700
Subject: [PATCH 3/5] Py formatting
---
.../API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index 215f8440cc68a..a8a6302d7cf13 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -107,7 +107,7 @@ def test_removing_and_adding_insertion_order(self):
def test_get_total_in_bytes(self):
"""
- Tests that get total in bytes properly returns an error without a
process,
+ Tests that get total in bytes properly returns an error without a
process,
and the readable regions with a process.
"""
>From ef3cf723251225f46b4e0bff980b86cd1d05c0bb Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Fri, 2 May 2025 14:36:12 -0700
Subject: [PATCH 4/5] Move to llvm::expected, clean up comments
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 4 ++--
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 9 ++++++++-
lldb/source/Symbol/SaveCoreOptions.cpp | 14 +++++++-------
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 2066ab3e8246c..37552c13d0f36 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -119,8 +119,8 @@ class LLDB_API SBSaveCoreOptions {
/// an empty collection will be returned.
SBThreadCollection GetThreadsToSave() const;
- /// Get the current total number of bytes the core is expected to be but not
- /// including the overhead of the core file format. Requires a Process and
+ /// Get the current total number of bytes the core is expected to have
+ /// excluding the overhead of the core file format. Requires a Process and
/// Style to be specified.
///
/// \note
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 319d44a6b0c87..da66b184745db 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -49,7 +49,7 @@ class SaveCoreOptions {
lldb_private::ThreadCollection::collection GetThreadsToSave() const;
- uint64_t GetCurrentSizeInBytes(Status &error);
+ llvm::Expected<uint64_t> GetCurrentSizeInBytes();
void Clear();
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index b67df513fe91b..410fb673b347a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -116,7 +116,14 @@ void SBSaveCoreOptions::Clear() {
uint64_t SBSaveCoreOptions::GetCurrentSizeInBytes(SBError &error) {
LLDB_INSTRUMENT_VA(this, error);
- return m_opaque_up->GetCurrentSizeInBytes(error.ref());
+ llvm::Expected<uint64_t> expected_bytes =
m_opaque_up->GetCurrentSizeInBytes();
+ if (!expected_bytes) {
+ error =
SBError(lldb_private::Status::FromError(expected_bytes.takeError()));
+ return 0;
+ }
+ // Clear the error, so if the clearer uses it we set it to success.
+ error.Clear();
+ return *expected_bytes;
}
lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index cf6fc99f7236e..9f4351fd54a7c 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -145,20 +145,20 @@ SaveCoreOptions::GetThreadsToSave() const {
return thread_collection;
}
-uint64_t SaveCoreOptions::GetCurrentSizeInBytes(Status &error) {
- if (!m_process_sp) {
- error = Status::FromErrorString("Requires a process to be set.");
- return 0;
- }
+llvm::Expected<uint64_t> SaveCoreOptions::GetCurrentSizeInBytes() {
+ Status error;
+ if (!m_process_sp)
+ return Status::FromErrorString("Requires a process to be
set.").takeError();
+
error = EnsureValidConfiguration(m_process_sp);
if (error.Fail())
- return 0;
+ return error.takeError();
CoreFileMemoryRanges ranges;
error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
if (error.Fail())
- return 0;
+ return error.takeError();
uint64_t total_in_bytes = 0;
for (auto &core_range : ranges)
>From 0f5c5a290fd49b84a64d0f266013b2ee5ab19501 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Fri, 2 May 2025 14:37:59 -0700
Subject: [PATCH 5/5] Add a new docstring
---
lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index 6efbe45d2d3ab..b676a00bd1113 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -63,6 +63,11 @@ Note that currently ELF Core files are not supported."
Get an SBThreadCollection of all threads marked to be saved. This
collection is not sorted according to insertion order."
) lldb::SBSaveCoreOptions::GetThreadsToSave;
+%feature("docstring", "
+ Get the current total number of bytes the core is expectd to have,
excluding the overhead of the core file format.
+ Requires both a Process and a Style to be specified."
+) lldb::SBSaveCoreOptions::GetCurrentSizeInBytes;
+
%feature("docstring", "
Unset all options."
) lldb::SBSaveCoreOptions::Clear;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
