Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLDB] Make the thread list for SBSaveCoreOptions iterable #122541

Merged
merged 8 commits into from
Jan 16, 2025
Merged

[LLDB] Make the thread list for SBSaveCoreOptions iterable #122541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5a756db
Make the thread list for SBSaveCoreOptions iterable
Jlalond Jan 10, 2025
7638844
Comment cleanup
Jlalond Jan 10, 2025
a1c2d9d
Refactor to a thread collection SP
Jlalond Jan 11, 2025
a93b515
Rephrase from collection to copy
Jlalond Jan 11, 2025
47c0c50
Uncomment tests
Jlalond Jan 11, 2025
d6d45ad
Save core doesn't store thread info, instead we get the threadlist fr…
Jlalond Jan 13, 2025
642f63c
Reorganize code per suggestion
Jlalond Jan 14, 2025
b765308
Pass back vector of thread sp and construct ThreadCollectionSP at the…
Jlalond Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lldb/include/lldb/API/SBSaveCoreOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "lldb/API/SBFileSpec.h"
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBThread.h"
#include "lldb/API/SBThreadCollection.h"

namespace lldb {

Expand Down Expand Up @@ -111,11 +112,19 @@ class LLDB_API SBSaveCoreOptions {
/// style specific regions.
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo &region);

/// Get an unsorted copy of all threads to save
///
/// \returns
/// An unsorted copy of all threads to save. If no process is specified
/// an empty collection will be returned.
SBThreadCollection GetThreadsToSave() const;

/// Reset all options.
void Clear();

protected:
friend class SBProcess;
friend class SBThreadCollection;
lldb_private::SaveCoreOptions &ref() const;

private:
Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/API/SBThreadCollection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class LLDB_API SBThreadCollection {
private:
friend class SBProcess;
friend class SBThread;

friend class SBSaveCoreOptions;
lldb::ThreadCollectionSP m_opaque_sp;
};

Expand Down
4 changes: 3 additions & 1 deletion lldb/include/lldb/Symbol/SaveCoreOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H

#include "lldb/Target/ThreadCollection.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/RangeMap.h"

#include <optional>
#include <set>
#include <string>
#include <unordered_set>

Expand Down Expand Up @@ -47,6 +47,8 @@ class SaveCoreOptions {

void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);

lldb_private::ThreadCollection::collection GetThreadsToSave() const;

void Clear();

private:
Expand Down
9 changes: 9 additions & 0 deletions lldb/source/API/SBSaveCoreOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "lldb/API/SBMemoryRegionInfo.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Target/ThreadCollection.h"
#include "lldb/Utility/Instrumentation.h"

#include "Utils.h"
Expand Down Expand Up @@ -100,6 +101,14 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo &region) {
return SBError();
}

lldb::SBThreadCollection SBSaveCoreOptions::GetThreadsToSave() const {
LLDB_INSTRUMENT_VA(this);
lldb::ThreadCollectionSP threadcollection_sp =
std::make_shared<lldb_private::ThreadCollection>(
m_opaque_up->GetThreadsToSave());
return SBThreadCollection(threadcollection_sp);
}

void SBSaveCoreOptions::Clear() {
LLDB_INSTRUMENT_VA(this);
m_opaque_up->Clear();
Expand Down
23 changes: 18 additions & 5 deletions lldb/source/Symbol/SaveCoreOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ void SaveCoreOptions::AddMemoryRegionToSave(
const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
return m_regions_to_save;
}

Status SaveCoreOptions::EnsureValidConfiguration(
lldb::ProcessSP process_sp) const {
Status
SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
Status error;
std::string error_str;
if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
Expand All @@ -132,10 +131,24 @@ Status SaveCoreOptions::EnsureValidConfiguration(
return error;
}

void SaveCoreOptions::ClearProcessSpecificData() {
lldb_private::ThreadCollection::collection
SaveCoreOptions::GetThreadsToSave() const {
lldb_private::ThreadCollection::collection thread_collection;
// In cases where no process is set, such as when no threads are specified.
if (!m_process_sp)
return thread_collection;

ThreadList &thread_list = m_process_sp->GetThreadList();
for (const auto &tid : m_threads_to_save)
thread_collection.push_back(thread_list.FindThreadByID(tid));

return thread_collection;
}

void SaveCoreOptions::ClearProcessSpecificData() {
// Deliberately not following the formatter style here to indicate that
// this method will be expanded in the future.
m_threads_to_save.clear();
m_threads_to_save.clear();
}

void SaveCoreOptions::Clear() {
Expand Down
31 changes: 28 additions & 3 deletions lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *


class SBSaveCoreOptionsAPICase(TestBase):
basic_minidump = "basic_minidump.yaml"
basic_minidump_different_pid = "basic_minidump_different_pid.yaml"

def get_process_from_yaml(self, yaml_file):
minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
print ("minidump_path: " + minidump_path)
print("minidump_path: " + minidump_path)
self.yaml2obj(yaml_file, minidump_path)
self.assertTrue(os.path.exists(minidump_path), "yaml2obj did not emit a minidump file")
self.assertTrue(
os.path.exists(minidump_path), "yaml2obj did not emit a minidump file"
)
target = self.dbg.CreateTarget(None)
process = target.LoadCore(minidump_path)
self.assertTrue(process.IsValid(), "Process is not valid")
Expand Down Expand Up @@ -59,7 +62,6 @@ def test_adding_and_removing_thread(self):
removed_success = options.RemoveThread(thread)
self.assertFalse(removed_success)


def test_adding_thread_different_process(self):
"""Test adding and removing a thread from save core options."""
options = lldb.SBSaveCoreOptions()
Expand All @@ -79,3 +81,26 @@ def test_adding_thread_different_process(self):
self.assertTrue(error.Fail())
error = options.AddThread(thread)
self.assertTrue(error.Success())

def test_removing_and_adding_insertion_order(self):
"""Test insertion order is maintained when removing and adding threads."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for SBSaveCoreOptions::GetThreadsToSave says you get an unsorted collection back. Which one is the intended behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Alex, good catch. This shouldn't test the ordering, it did before when used our own internal collection but after Greg and I had some back and forth now we're returning a thread collection.

options = lldb.SBSaveCoreOptions()
process = self.get_basic_process()
threads = []
for x in range(0, 3):
thread = process.GetThreadAtIndex(x)
threads.append(thread)
error = options.AddThread(thread)
self.assertTrue(error.Success())

# Get the middle thread, remove it, and insert it back.
middle_thread = threads[1]
self.assertTrue(options.RemoveThread(middle_thread))
thread_collection = options.GetThreadsToSave()
self.assertTrue(thread_collection is not None)
self.assertEqual(thread_collection.GetSize(), 2)
error = options.AddThread(middle_thread)
self.assertTrue(error.Success())
thread_collection = options.GetThreadsToSave()
self.assertEqual(thread_collection.GetSize(), 3)
self.assertIn(middle_thread, thread_collection)
10 changes: 10 additions & 0 deletions lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,13 @@ Streams:
Stack:
Start of Memory Range: 0x00007FFFC8D0E000
Content: 'DEADBEEF'
- Thread Id: 0x000074DE
Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Stack:
Start of Memory Range: 0x00007FFFC8D0A000
Content: 'BEEFDEAD'
- Thread Id: 0x000074DF
Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Stack:
Start of Memory Range: 0x00007FFFC8DFF000
Content: 'BAADBEEF'
Loading