[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -602,17 +603,17 @@ Status MinidumpFileBuilder::AddThreadList() {
return error;
llvm::support::ulittle32_t thread_count =
- static_cast(thread_list.GetSize());
+ static_cast(thread_list.size());
m_data.AppendData(_count, sizeof(llvm::support::ulittle32_t));
// Take the offset after the thread count.
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
- const uint32_t num_threads = thread_list.GetSize();
+ const uint32_t num_threads = thread_list.size();
clayborg wrote:
remove
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -6730,7 +6731,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
_sp,
StructuredData::ArraySP threads(
std::make_shared());
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ ThreadSP thread_sp = thread_list.at(thread_idx);
clayborg wrote:
```
for (ThreadSP thread_sp : thread_list) {
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+ }
+
+ if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+ }
+
+ if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+ m_process_sp = process_sp;
+ return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+ Status error;
+ if (!thread) {
+error.SetErrorString("Thread is null");
+ }
+
+ if (!m_process_sp.has_value())
+m_process_sp = thread->GetProcess();
+
+ if (m_process_sp.value() != thread->GetProcess()) {
+error.SetErrorString("Cannot add thread from a different process.");
+return error;
+ }
clayborg wrote:
```
if (m_process_sp) {
if (m_process_sp != thread->GetProcess()) {
error.SetErrorString("Cannot add thread from a different process.");
return error;
}
} else {
m_process_sp = thread->GetProcess();
}
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+ }
+
+ if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+ }
+
+ if (m_process_sp.has_value())
clayborg wrote:
m_process_sp should not be optional as the shared pointer being NULL is enough.
This code now becomes:
```
if (m_process_sp)
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+ }
+
+ if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+ }
+
+ if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+ m_process_sp = process_sp;
+ return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+ Status error;
+ if (!thread) {
clayborg wrote:
early return like Jason suggested and this will now be:
```
if (!thread_sp) {
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -53,6 +54,26 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Set the process to save, or unset if supplied with a null process.
+ ///
+ /// \param process The process to save.
+ /// \return Success if process was set, otherwise an error
+ /// \note This will clear all process specific options if
+ /// an exisiting process is overriden.
+ SBError SetProcess(lldb::SBProcess process);
+
+ /// Add a thread to save in the core file.
+ ///
+ /// \param thread The thread to save.
+ /// \note This will set the process if it is not already set.
clayborg wrote:
And mention that it will return an error if the process is already set and the
SBThread doesn't match the current process
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+ }
+
+ if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+ }
+
+ if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+ m_process_sp = process_sp;
+ return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+ Status error;
+ if (!thread) {
+error.SetErrorString("Thread is null");
+ }
+
+ if (!m_process_sp.has_value())
+m_process_sp = thread->GetProcess();
+
+ if (m_process_sp.value() != thread->GetProcess()) {
+error.SetErrorString("Cannot add thread from a different process.");
+return error;
+ }
+
+ std::pair tid_pair(thread->GetID(),
+ thread->GetBackingThread());
clayborg wrote:
We should switch the parameter to a `ThreadSP`. But FYI: `lldb_private::Thread`
inherits from `std::enable_shared_from_this` which means you can call
`thread->shared_from_this()` to get a shared pointer. All of our objects that
are held in shared pointers inherit from `std::enable_shared_from_this` to
allow them to do this.
you can change this code to:
```
m_threads_to_save[thread_sp->GetID()] = thread_sp;
```
and remove the `insert` call below.
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
clayborg wrote:
Don't use optionals for std::shared_ptr as it being NULL is enough. This then
becomes:
```
m_process_sp.reset();
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -602,17 +603,17 @@ Status MinidumpFileBuilder::AddThreadList() {
return error;
llvm::support::ulittle32_t thread_count =
- static_cast(thread_list.GetSize());
+ static_cast(thread_list.size());
m_data.AppendData(_count, sizeof(llvm::support::ulittle32_t));
// Take the offset after the thread count.
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
- const uint32_t num_threads = thread_list.GetSize();
+ const uint32_t num_threads = thread_list.size();
Log *log = GetLog(LLDBLog::Object);
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ThreadSP thread_sp = thread_list.at(thread_idx);
clayborg wrote:
```
for (ThreadSP thread_sp : thread_list) {
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/100443 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -6622,7 +6623,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
_sp,
LC_THREAD_data.SetByteOrder(byte_order);
}
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ ThreadSP thread_sp = thread_list.at(thread_idx);
clayborg wrote:
Replace these three lines with:
```
for (ThreadSP thread_sp : thread_list) {
```
No need to use indexes before since we have an STL container which supports
iteration.
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -32,12 +33,24 @@ class SaveCoreOptions {
void SetOutputFile(lldb_private::FileSpec file);
const std::optional GetOutputFile() const;
+ Status SetProcess(lldb::ProcessSP process_sp);
+
+ Status AddThread(lldb_private::Thread *thread);
+ bool RemoveThread(lldb_private::Thread *thread);
+ bool ShouldSaveThread(lldb::tid_t tid) const;
+
+ Status EnsureValidConfiguration(lldb::ProcessSP process_to_save) const;
+
void Clear();
private:
+ void ClearProcessSpecificData();
+
std::optional m_plugin_name;
std::optional m_file;
std::optional m_style;
+ std::optional m_process_sp;
clayborg wrote:
This is a shared pointer, it being NULL is enough to say that there is no
process set. Switch this to just a `ProcessSP m_process_sp;`
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -6608,8 +6608,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP _sp, mach_header.ncmds = segment_load_commands.size(); mach_header.flags = 0; mach_header.reserved = 0; -ThreadList _list = process_sp->GetThreadList(); -const uint32_t num_threads = thread_list.GetSize(); +std::vector thread_list = +process_sp->CalculateCoreFileThreadList(options); +const uint32_t num_threads = thread_list.size(); clayborg wrote: remove this as before we needed to use the `ThreadList` class which didn't have an iterator. Now we have a `std::vector` so we can use the build in iteration. See below. https://github.com/llvm/llvm-project/pull/100443 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+ }
+
+ return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+ return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+ auto iter = m_threads_to_save.begin();
+ while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+ return *iter;
+index--;
+iter++;
+ }
+
+ return -1;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+ // If the user specified no threads to save, then we save all threads.
+ if (m_threads_to_save.empty())
+return true;
+ return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration() const {
+ Status error;
+ std::string error_str;
+ if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+error_str += "Cannot save a full core with a subset of threads\n";
clayborg wrote:
Should we allow "full" core files to be emitted without some thread stacks? We
could allow "full" to mean save all memory regions except the thread stacks for
any threads that were not in the list. This would allow core files to be a bit
smaller, but still contain all mapped memory except the thread stacks we didn't
want. We can emit a warning when saving a core file saying something to this
effect like we do for "stacks" and "modified-memory"
The reason I say this is for core files for and Apple systems. If you save a
`full` style, all mach-o binaries are fully mapped into memory and you would
have everything you need to load the core file as all system libraries are
mapped into memory, and it would be nice to be able to not save all thread
stacks if you don't need them. So maybe turn this into a warning we can expose
to the user
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+ }
+
+ return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+ return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+ auto iter = m_threads_to_save.begin();
+ while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+ return *iter;
+index--;
+iter++;
+ }
clayborg wrote:
If this list is large, then this function can be quite inefficient. But
probably ok for now.
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -32,12 +33,21 @@ class SaveCoreOptions {
void SetOutputFile(lldb_private::FileSpec file);
const std::optional GetOutputFile() const;
+ void AddThread(lldb::tid_t tid);
+ bool RemoveThread(lldb::tid_t tid);
+ size_t GetNumThreads() const;
+ int64_t GetThreadAtIndex(size_t index) const;
+ bool ShouldSaveThread(lldb::tid_t tid) const;
+
+ Status EnsureValidConfiguration() const;
+
void Clear();
private:
std::optional m_plugin_name;
std::optional m_file;
std::optional m_style;
+ std::set m_threads_to_save;
clayborg wrote:
FYI: Totally fine to save things internally by lldb::tid_t since we know and
control this code.
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+ }
+
+ return false;
clayborg wrote:
erase returns a bool if something was erased, lines 55-60 can just be:
```
return m_threads_to_save.erase(tid) > 0;
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+ }
clayborg wrote:
FYI this would fail to remove the tid because you have
`m_threads_to_save.count(tid) == 0` instead of `m_threads_to_save.count(tid) !=
0`, but see the one line replacement code that will work below.
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Add a thread to save in the core file.
+ ///
+ /// \param thread_id The thread ID to save.
+ void AddThread(lldb::tid_t thread_id);
+
+ /// Remove a thread from the list of threads to save.
+ ///
+ /// \param thread_id The thread ID to remove.
+ /// \return True if the thread was removed, false if it was not in the list.
+ bool RemoveThread(lldb::tid_t thread_id);
clayborg wrote:
Use `lldb::SBThread` instead of `lldb::tid_t`
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
clayborg wrote:
No need to check the count, and no need to use `emplace` when we have a
`std::set` and the type `T` is an integer:
```
m_threads_to_save.insert(tid);
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+ }
+
+ return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+ return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+ auto iter = m_threads_to_save.begin();
+ while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+ return *iter;
+index--;
+iter++;
+ }
+
+ return -1;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+ // If the user specified no threads to save, then we save all threads.
+ if (m_threads_to_save.empty())
+return true;
+ return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration() const {
+ Status error;
+ std::string error_str;
+ if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+error_str += "Cannot save a full core with a subset of threads\n";
+ }
clayborg wrote:
Remove `{}` on single line if statement per llvm coding guidelines
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -32,12 +33,21 @@ class SaveCoreOptions {
void SetOutputFile(lldb_private::FileSpec file);
const std::optional GetOutputFile() const;
+ void AddThread(lldb::tid_t tid);
+ bool RemoveThread(lldb::tid_t tid);
+ size_t GetNumThreads() const;
+ int64_t GetThreadAtIndex(size_t index) const;
clayborg wrote:
This should be:
```
lldb::tid_t GetThreadAtIndex(size_t index) const;
```
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Add a thread to save in the core file.
+ ///
+ /// \param thread_id The thread ID to save.
+ void AddThread(lldb::tid_t thread_id);
+
+ /// Remove a thread from the list of threads to save.
+ ///
+ /// \param thread_id The thread ID to remove.
+ /// \return True if the thread was removed, false if it was not in the list.
+ bool RemoveThread(lldb::tid_t thread_id);
+
+ /// Get the number of threads to save. If this list is empty all threads will
+ /// be saved.
+ ///
+ /// \return The number of threads to save.
+ uint32_t GetNumThreads() const;
+
+ /// Get the thread ID at the given index.
+ ///
+ /// \param[in] index The index of the thread ID to get.
+ /// \return The thread ID at the given index, or an error
+ /// if there is no thread at the index.
+ lldb::tid_t GetThreadAtIndex(uint32_t index, SBError ) const;
clayborg wrote:
returning a `lldb::SBThread`
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Add a thread to save in the core file.
+ ///
+ /// \param thread_id The thread ID to save.
+ void AddThread(lldb::tid_t thread_id);
clayborg wrote:
Do we want to do things by `lldb::tid_t` or do we want to pass in a
`lldb::SBThread`? The reason I mention this is there are two integer
identifiers from a SBThread:
```
lldb::tid_t lldb::SBThread::GetThreadID() const;
uint32_t lldb::SBThread::GetIndexID() const;
```
This API could and probably should be:
```
void AddThread(lldb::SBThread thread);
```
Because in order to get a lldb::tid_t users would need to have a SBThread
already anyway. Then the user can't mess up by specifying the wrong ID (index
ID instead of `lldb::tid_t`)
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/100443 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)
https://github.com/clayborg commented: Update the public API to user SBThread instead of the thread IDs and then add some tests. https://github.com/llvm/llvm-project/pull/100443 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (PR #99362)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/99362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/99736 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -158,7 +158,7 @@ class LLDB_API SBProcess {
lldb::SBError Destroy();
- lldb::SBError Continue();
+ lldb::SBError Continue(RunDirection direction = RunDirection::eRunForward);
clayborg wrote:
Our public API has rules:
- can't change any existing API call, you can add an overload, but you can't
change any public APIs that are already there. Anything in the `lldb` namespace
can't be changed.
- no virtual functions
- can't change any ivars or the size of the object
That being said, I would rather have a:
```
lldb::SBError ReverseContinue();
```
call than have everyone using the `Continue` API to say wether they want to go
forward or in reverse.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -1129,10 +1129,15 @@ class Process : public
std::enable_shared_from_this,
/// \see Thread:Resume()
/// \see Thread:Step()
/// \see Thread:Suspend()
- virtual Status DoResume() {
+ virtual Status DoResume(lldb::RunDirection direction) {
clayborg wrote:
I would rather have a new `DoResumeReverse()` instead of changing this virtual
API. Many modified files in this patch are a result of the process plugins
having to add support for not supporting reverse resumes.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -135,6 +135,9 @@ FLAGS_ENUM(LaunchFlags){
/// Thread Run Modes.
enum RunMode { eOnlyThisThread, eAllThreads, eOnlyDuringStepping };
+/// Execution directions
+enum RunDirection { eRunForward, eRunReverse };
+
clayborg wrote:
If we don't add an overload to continue by adding
`SBProcess::ReverseContinue()` then this should be kept internal and not in
this header file. If you add a new `SBProcess::Continue(lldb::RunDirection)`
overload then this is needed. I would prefer a dedicated
`SBProcess::ReverseContinue()` function.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -203,11 +203,17 @@ ProcessWindows::DoAttachToProcessWithID(lldb::pid_t pid,
return error;
}
-Status ProcessWindows::DoResume() {
+Status ProcessWindows::DoResume(RunDirection direction) {
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -52,7 +52,7 @@ class ProcessWindows : public Process, public ProcessDebugger
{
Status DoAttachToProcessWithID(
lldb::pid_t pid,
const lldb_private::ProcessAttachInfo _info) override;
- Status DoResume() override;
+ Status DoResume(lldb::RunDirection direction) override;
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -402,9 +402,16 @@ lldb_private::DynamicLoader
*ProcessKDP::GetDynamicLoader() {
Status ProcessKDP::WillResume() { return Status(); }
-Status ProcessKDP::DoResume() {
+Status ProcessKDP::DoResume(RunDirection direction) {
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -52,7 +52,7 @@ class ScriptedProcess : public Process {
void DidResume() override;
- Status DoResume() override;
+ Status DoResume(lldb::RunDirection direction) override;
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -203,11 +203,17 @@ ProcessWindows::DoAttachToProcessWithID(lldb::pid_t pid,
return error;
}
-Status ProcessWindows::DoResume() {
+Status ProcessWindows::DoResume(RunDirection direction) {
Log *log = GetLog(WindowsLog::Process);
llvm::sys::ScopedLock lock(m_mutex);
Status error;
+ if (direction == RunDirection::eRunReverse) {
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -182,10 +182,17 @@ void ScriptedProcess::DidResume() {
m_pid = GetInterface().GetProcessID();
}
-Status ScriptedProcess::DoResume() {
+Status ScriptedProcess::DoResume(RunDirection direction) {
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -874,10 +874,10 @@ class Process : public std::enable_shared_from_this, /// \see Thread:Resume() /// \see Thread:Step() /// \see Thread:Suspend() - Status Resume(); + Status Resume(lldb::RunDirection direction = lldb::eRunForward); clayborg wrote: internal APIs, anything not in the `lldb` namespace _can_ be changed. So this is ok. Though I personally would like to see a: ``` Status ReverseResume(); ``` I am open to feedback from other here as my mind can easily be changed. https://github.com/llvm/llvm-project/pull/99736 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/99736 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -90,7 +90,7 @@ class ProcessKDP : public lldb_private::Process {
// Process Control
lldb_private::Status WillResume() override;
- lldb_private::Status DoResume() override;
+ lldb_private::Status DoResume(lldb::RunDirection direction) override;
clayborg wrote:
We should use `DoResumeReverse()` in Process.h and this change won't need to
happen.
https://github.com/llvm/llvm-project/pull/99736
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -111,7 +111,7 @@ class ProcessGDBRemote : public Process, // Process Control Status WillResume() override; - Status DoResume() override; + Status DoResume(lldb::RunDirection direction) override; clayborg wrote: We should use `DoResumeReverse()` in Process.h and this change will add a new interface definition. https://github.com/llvm/llvm-project/pull/99736 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #99736)
@@ -3139,6 +3146,7 @@ void PruneThreadPlans(); // m_currently_handling_do_on_removals are true, // Resume will only request a resume, using this // flag to check. + lldb::RunDirection m_last_run_direction; clayborg wrote: Feel free to initialize this here so we don't have to change the constructor: ``` lldb::RunDirection m_last_run_direction = lldb::eRunForward; ``` https://github.com/llvm/llvm-project/pull/99736 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCoreOptions] Fix TestProcessSaveCore (PR #99692)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/99692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (PR #98403)
https://github.com/clayborg approved this pull request. Thanks for sticking with me, this looks really good now https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,69 @@ +//===-- SBSaveCoreOptions.h -*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_API_SBSaveCoreOPTIONS_H +#define LLDB_API_SBSaveCoreOPTIONS_H clayborg wrote: this should be all upper case https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -1216,13 +1217,26 @@ bool SBProcess::IsInstrumentationRuntimePresent(
lldb::SBError SBProcess::SaveCore(const char *file_name) {
LLDB_INSTRUMENT_VA(this, file_name);
- return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull);
+ SBSaveCoreOptions options;
+ options.SetOutputFile(SBFileSpec(file_name));
+ options.SetStyle(SaveCoreStyle::eSaveCoreFull);
+ return SaveCore(options);
}
lldb::SBError SBProcess::SaveCore(const char *file_name,
const char *flavor,
SaveCoreStyle core_style) {
LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style);
+ SBSaveCoreOptions options;
+ options.SetOutputFile(SBFileSpec(file_name));
+ options.SetPluginName(flavor);
clayborg wrote:
Check the error from SetPluginName here and return an error if it fails.
Otherwise the option won't get set due to the error and it will try to default
to the default plug-in.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (PR #98403)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (PR #98403)
https://github.com/clayborg commented: Just two quick things and this is good to go! https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,44 @@
+//===-- CoreDumpOptions.h ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H
+#define LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+
+#include
+#include
+
+namespace lldb_private {
+
+class CoreDumpOptions {
clayborg wrote:
If we rename `SBCoreDumpOptions` to `SBSaveCoreOptions`, this should become
`SaveCoreOptions`
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,52 @@
+//===-- CoreDumpOptions.cpp -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Core/PluginManager.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Status CoreDumpOptions::SetPluginName(const char *name) {
+ Status error;
+ if (!name || !name[0]) {
+m_plugin_name = std::nullopt;
+ }
clayborg wrote:
remove braces from single statement `if` per llvm coding guidelines, or add a
`return error;` inside the `if` statement.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -355,11 +355,10 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
}
bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP _sp,
-const lldb_private::FileSpec ,
-lldb::SaveCoreStyle _style,
+const lldb_private::CoreDumpOptions ,
lldb_private::Status ) {
- core_style = eSaveCoreFull;
- return SaveMiniDump(process_sp, outfile, error);
+ assert(options.GetOutputFile().has_value());
clayborg wrote:
Add same comment and add `assert(process_sp);`
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,69 @@
+//===-- SBCoreDumpOptions.h -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_API_SBCOREDUMPOPTIONS_H
+#define LLDB_API_SBCOREDUMPOPTIONS_H
+
+#include "lldb/API/SBDefines.h"
+#include "lldb/Symbol/CoreDumpOptions.h"
+
+namespace lldb {
+
+class LLDB_API SBCoreDumpOptions {
clayborg wrote:
Should we change the name to `SBSaveCoreOptions`? `SBCoreDumpOptions` seems
like they would be used to dump a core dump to the terminal?
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -6519,14 +6519,15 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP _sp,
- const FileSpec ,
- lldb::SaveCoreStyle _style, Status )
{
- if (!process_sp)
-return false;
-
- // Default on macOS is to create a dirty-memory-only corefile.
+ const lldb_private::CoreDumpOptions ,
+ Status ) {
+ auto core_style = options.GetStyle();
if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
+ // The FileSpec and Process are already checked in PluginManager::SaveCore.
+ assert(options.GetOutputFile().has_value());
+ const FileSpec outfile = options.GetOutputFile().value();
+ assert(process_sp);
clayborg wrote:
move this up a line so the comment makes more sense.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,86 @@
+//===-- SBCoreDumpOptions.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBCoreDumpOptions.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Utility/Instrumentation.h"
+
+#include "Utils.h"
+
+using namespace lldb;
+
+SBCoreDumpOptions::SBCoreDumpOptions() {
+ LLDB_INSTRUMENT_VA(this)
+
+ m_opaque_up = std::make_unique();
+}
+
+SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+const SBCoreDumpOptions &
+SBCoreDumpOptions::operator=(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ if (this != )
+m_opaque_up = clone(rhs.m_opaque_up);
+ return *this;
+}
+
+SBError SBCoreDumpOptions::SetPluginName(const char *name) {
+ LLDB_INSTRUMENT_VA(this, name);
+ lldb_private::Status error = m_opaque_up->SetPluginName(name);
+ return SBError(error);
+}
+
+void SBCoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) {
+ LLDB_INSTRUMENT_VA(this, style);
+ m_opaque_up->SetStyle(style);
+}
+
+void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec file_spec) {
+ LLDB_INSTRUMENT_VA(this, file_spec);
+ m_opaque_up->SetOutputFile(file_spec.ref());
+}
+
+const char *SBCoreDumpOptions::GetPluginName() const {
+ LLDB_INSTRUMENT_VA(this);
+ const auto name = m_opaque_up->GetPluginName();
+ if (!name)
+return nullptr;
+ return lldb_private::ConstString(name.value()).GetCString();
+}
+
+SBFileSpec SBCoreDumpOptions::GetOutputFile() const {
+ LLDB_INSTRUMENT_VA(this);
+ const auto file_spec = m_opaque_up->GetOutputFile();
+ if (file_spec)
+return SBFileSpec(file_spec.value());
+ return SBFileSpec();
+}
+
+lldb::SaveCoreStyle SBCoreDumpOptions::GetStyle() const {
+ LLDB_INSTRUMENT_VA(this);
+ return m_opaque_up->GetStyle();
+}
+
+lldb_private::CoreDumpOptions ::ref() const {
+ LLDB_INSTRUMENT_VA(this);
clayborg wrote:
This is an internal API we don't need to log... Anything private or protected
doesn't need to be instrumented.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -689,30 +702,38 @@
PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
}
Status PluginManager::SaveCore(const lldb::ProcessSP _sp,
- const FileSpec ,
- lldb::SaveCoreStyle _style,
- llvm::StringRef plugin_name) {
- if (plugin_name.empty()) {
+ const lldb_private::CoreDumpOptions ) {
+ Status error;
+ if (!options.GetOutputFile()) {
+error.SetErrorString("No output file specified");
+return error;
+ }
+
clayborg wrote:
You added `assert(process_sp)` in the object file plugins, but didn't check the
process here. Add:
```
if (!process_sp) {
error.SetErrorString("Invalid process");
return error;
}
```
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,55 @@
+//===-- CoreDumpOptions.cpp -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Core/PluginManager.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Status CoreDumpOptions::SetPluginName(const char *name) {
+ Status error;
+ if (!name || !name[0]) {
+m_plugin_name = std::nullopt;
+error.SetErrorString("no plugin name specified");
+ }
+
+ if (!PluginManager::IsRegisteredPluginName(name)) {
+error.SetErrorStringWithFormat(
+"plugin name '%s' is not a registered plugin", name);
clayborg wrote:
"plugin name '%s' is not a valid ObjectFile plugin name"
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -639,6 +640,18 @@ static ObjectFileInstances () {
return g_instances;
}
+bool PluginManager::IsRegisteredPluginName(const char *name) {
+ if (!name || !name[0])
clayborg wrote:
Ask the `llvm::StringRef` if is it empty:
```
if (name.empty())
```
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -6519,14 +6519,16 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP _sp,
- const FileSpec ,
- lldb::SaveCoreStyle _style, Status )
{
- if (!process_sp)
-return false;
-
- // Default on macOS is to create a dirty-memory-only corefile.
+ const lldb_private::CoreDumpOptions ,
+ Status ) {
+ auto core_style = options.GetStyle();
if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
+ // The FileSpec is already checked in PluginManager::SaveCore.
+ assert(options.GetOutputFile().has_value());
+ const FileSpec outfile = options.GetOutputFile().value();
+ if (!process_sp)
+return false;
clayborg wrote:
This would be checked in the `PluginManager::SaveCore(...)` and we can do a
assert like we did for the options.GetOutputFile() above
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,76 @@
+//===-- SBCoreDumpOptions.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBCoreDumpOptions.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Utility/Instrumentation.h"
+
+#include "Utils.h"
+
+using namespace lldb;
+
+SBCoreDumpOptions::SBCoreDumpOptions() {
+ LLDB_INSTRUMENT_VA(this)
+
+ m_opaque_up = std::make_unique();
+}
+
+SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+const SBCoreDumpOptions &
+SBCoreDumpOptions::operator=(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ if (this != )
+m_opaque_up = clone(rhs.m_opaque_up);
+ return *this;
+}
+
+SBError SBCoreDumpOptions::SetPluginName(const char *name) {
+ lldb_private::Status error = m_opaque_up->SetPluginName(name);
clayborg wrote:
Add LLDB_INSTRUMENT_VA here and too all methods.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,25 @@ +"""Test the SBCoreDumpOptions APIs.""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class SBCoreDumpOptionsAPICase(TestBase): +def test_plugin_name_assignment(self): +"""Test""" +options = lldb.SBCoreDumpOptions() +error = options.SetPluginName(None) +self.assertTrue(error.Fail()) clayborg wrote: This shouldn't fail, setting it to None clears the setting. So this test should probably set it to something valid first, like "minidump", verify you get that name back, then set it to None, verify you get None back. https://github.com/llvm/llvm-project/pull/98403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,55 @@
+//===-- CoreDumpOptions.cpp -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Core/PluginManager.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Status CoreDumpOptions::SetPluginName(const char *name) {
+ Status error;
+ if (!name || !name[0]) {
+m_plugin_name = std::nullopt;
+error.SetErrorString("no plugin name specified");
clayborg wrote:
This isn't an error, but a way to clear the specified plug-in name. So no error
here, just clear the plug-in name and that is a success
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -1256,7 +1256,7 @@ class CommandObjectProcessSaveCore : public
CommandObjectParsed {
class CommandOptions : public Options {
public:
-CommandOptions() = default;
+CommandOptions(){};
clayborg wrote:
revert this to use "= default;". The code is the same, but not need to change a
line of code if we don't have to.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,55 @@
+//===-- CoreDumpOptions.cpp -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Core/PluginManager.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Status CoreDumpOptions::SetPluginName(const char *name) {
+ Status error;
+ if (!name || !name[0]) {
+m_plugin_name = std::nullopt;
+error.SetErrorString("no plugin name specified");
+ }
+
+ if (!PluginManager::IsRegisteredPluginName(name)) {
+error.SetErrorStringWithFormat(
+"plugin name '%s' is not a registered plugin", name);
+return error;
+ }
+
+ m_plugin_name = name;
+ return error;
+}
+
+void CoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; }
+
+void CoreDumpOptions::SetOutputFile(FileSpec file) { m_file = file; }
+
+std::optional CoreDumpOptions::GetPluginName() const {
+ return m_plugin_name;
+}
+
+lldb::SaveCoreStyle CoreDumpOptions::GetStyle() const {
+ if (!m_style.has_value())
+return lldb::eSaveCoreUnspecified;
+ return m_style.value();
clayborg wrote:
These three lines can be:
```
return m_style.value_or(lldb::eSaveCoreUnspecified);
```
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -639,6 +640,18 @@ static ObjectFileInstances () {
return g_instances;
}
+bool PluginManager::IsRegisteredPluginName(const char *name) {
clayborg wrote:
`name` will now be a `llvm::StringRef`
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -56,18 +56,20 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
}
bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP _sp,
- const lldb_private::FileSpec ,
- lldb::SaveCoreStyle _style,
+ const lldb_private::CoreDumpOptions ,
lldb_private::Status ) {
- // Set default core style if it isn't set.
- if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
-core_style = SaveCoreStyle::eSaveCoreStackOnly;
-
+ assert(options.GetOutputFile().has_value());
if (!process_sp)
return false;
clayborg wrote:
Might as well check this in the plugin manager and assert(process_sp) with an
appropriate comment like we did above.
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -178,6 +178,8 @@ class PluginManager {
static bool UnregisterPlugin(ObjectFileCreateInstance create_callback);
+ static bool IsRegisteredPluginName(const char *name);
clayborg wrote:
This function name needs to be more complete to say we are searching for a
valid object file plug-in name because we have many different kinds of plugins
use the same type as the way names are stored, so use `llvm::StringRef`:
```
static bool IsRegisteredObjectFilePluginName(llvm::StringRef name);
```
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches (PR #99305)
https://github.com/clayborg commented: Anytime we change this code it makes me nervous. The unit tests are quite simple and not sure how they match up against real world lookups, but as far as I can tell this looks ok. https://github.com/llvm/llvm-project/pull/99305 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/98845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/98845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/98845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -2545,6 +2546,11 @@ ModuleSP Process::ReadModuleFromMemory(const FileSpec
_spec,
ModuleSP module_sp(new Module(file_spec, ArchSpec()));
if (module_sp) {
Status error;
+std::unique_ptr progress_up;
+if (!GetCoreFile())
clayborg wrote:
`PostMortemProcess` is what all of the core file processes (elf, mach-o and
minidump) inherit from. That way all core file processes don't forget to
override this function. I agree the name isn't the most clear.
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -195,20 +196,40 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
Target = process->GetTarget();
Status error;
+ StreamString prog_str;
+ if (!name.empty()) {
+prog_str << name.str() << " ";
+ }
+ if (uuid.IsValid())
+prog_str << uuid.GetAsString();
+ if (value_is_offset == 0 && value != LLDB_INVALID_ADDRESS) {
+prog_str << "at 0x";
+prog_str.PutHex64(value);
+ }
+
if (!uuid.IsValid() && !value_is_offset) {
+Progress progress_memread("Reading load commands from memory",
+ prog_str.GetString().str());
memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
-if (memory_module_sp)
+if (memory_module_sp) {
uuid = memory_module_sp->GetUUID();
+ if (uuid.IsValid()) {
+prog_str << " ";
+prog_str << uuid.GetAsString();
+ }
+}
}
ModuleSpec module_spec;
module_spec.GetUUID() = uuid;
FileSpec name_filespec(name);
- if (FileSystem::Instance().Exists(name_filespec))
-module_spec.GetFileSpec() = name_filespec;
if (uuid.IsValid()) {
+Progress progress("Locating external symbol file",
+ prog_str.GetString().str());
+
// Has lldb already seen a module with this UUID?
+// Or have external lookup enabled in DebugSymbols on macOS.
clayborg wrote:
My only concern here is this will spam the progress notifications and also be a
bit misleading. The title says "Locating external symbol file" when what this
is actually doing is loading the module from a module_spec. It _might_ end up
locating an external symbol file. We know reading modules from memory is slow,
so fine to make a progress for this, but It would be better to actually put
this in the `DownloadObjectAndSymbolFile()` with this label, or change the
label to something more like "Loading module" with the detail being the
`prog_str`
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -2545,6 +2546,11 @@ ModuleSP Process::ReadModuleFromMemory(const FileSpec
_spec,
ModuleSP module_sp(new Module(file_spec, ArchSpec()));
if (module_sp) {
Status error;
+std::unique_ptr progress_up;
+if (!GetCoreFile())
clayborg wrote:
Might be better to use the bool accessor function:
```
if (IsLiveDebugSession())
```
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][RISCV] function prologue backtrace fix (PR #99043)
https://github.com/clayborg commented: LGTM https://github.com/llvm/llvm-project/pull/99043 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][RISCV] function prologue backtrace fix (PR #99043)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/99043 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][RISCV] function prologue backtrace fix (PR #99043)
@@ -643,9 +644,9 @@ bool
ABISysV_riscv::CreateFunctionEntryUnwindPlan(UnwindPlan _plan) {
unwind_plan.Clear();
unwind_plan.SetRegisterKind(eRegisterKindDWARF);
clayborg wrote:
Since we specify here that we are using DWARF registers, then the change below
is correct,
https://github.com/llvm/llvm-project/pull/99043
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add frame recognizer for __builtin_verbose_trap (PR #80368)
clayborg wrote: I am fine with landing as is. Maybe we should add some `TODO:` comments in the 3 frame recognizers with comments saying what we really should do to put these kinds of frame plug-ins in the right places. https://github.com/llvm/llvm-project/pull/80368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -757,11 +758,32 @@ bool
DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
const ModuleList _images = target.GetImages();
m_module_sp = target_images.FindModule(m_uuid);
+StreamString prog_str;
+// 'mach_kernel' is a fake name we make up to find kernels
+// that were located by the local filesystem scan.
+if (GetName() != "mach_kernel")
+ prog_str << GetName() << " ";
+if (GetUUID().IsValid())
+ prog_str << GetUUID().GetAsString() << " ";
+if (GetLoadAddress() != LLDB_INVALID_ADDRESS) {
+ prog_str << "at 0x";
+ prog_str.PutHex64(GetLoadAddress());
+}
+std::unique_ptr progress_wp;
+if (IsKernel())
+ progress_wp = std::make_unique("Loading kernel",
+ prog_str.GetString().str());
+else
+ progress_wp = std::make_unique("Loading kext",
+ prog_str.GetString().str());
+
// Search for the kext on the local filesystem via the UUID
if (!m_module_sp && m_uuid.IsValid()) {
ModuleSpec module_spec;
module_spec.GetUUID() = m_uuid;
- module_spec.GetArchitecture() = target.GetArchitecture();
+ if (!m_uuid.IsValid())
+module_spec.GetArchitecture() = target.GetArchitecture();
clayborg wrote:
Did you mean to check if the UUID is valid in the if statement and then set the
architecture?
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -195,20 +196,40 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
Target = process->GetTarget();
Status error;
+ StreamString prog_str;
+ if (!name.empty()) {
+prog_str << name.str() << " ";
+ }
+ if (uuid.IsValid())
+prog_str << uuid.GetAsString();
+ if (value_is_offset == 0 && value != LLDB_INVALID_ADDRESS) {
+prog_str << "at 0x";
+prog_str.PutHex64(value);
+ }
+
if (!uuid.IsValid() && !value_is_offset) {
+Progress progress_memread("Reading load commands from memory",
clayborg wrote:
I would change this string to "Reading object file from memory" instead of
"Reading load commands from memory". This function is currently only used by
Mach-O core files and the GDB remote stub, but this could be used for loading
ELF files from memory as well.
Maybe this Progress dialog should go into `ReadUnnamedMemoryModule` so anyone
that calls this function gets progress events when loading an object file from
memory?
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
@@ -195,20 +196,40 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
Target = process->GetTarget();
Status error;
+ StreamString prog_str;
+ if (!name.empty()) {
+prog_str << name.str() << " ";
+ }
+ if (uuid.IsValid())
+prog_str << uuid.GetAsString();
+ if (value_is_offset == 0 && value != LLDB_INVALID_ADDRESS) {
+prog_str << "at 0x";
+prog_str.PutHex64(value);
+ }
+
if (!uuid.IsValid() && !value_is_offset) {
+Progress progress_memread("Reading load commands from memory",
+ prog_str.GetString().str());
memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
-if (memory_module_sp)
+if (memory_module_sp) {
uuid = memory_module_sp->GetUUID();
+ if (uuid.IsValid()) {
+prog_str << " ";
+prog_str << uuid.GetAsString();
+ }
+}
}
ModuleSpec module_spec;
module_spec.GetUUID() = uuid;
FileSpec name_filespec(name);
- if (FileSystem::Instance().Exists(name_filespec))
-module_spec.GetFileSpec() = name_filespec;
if (uuid.IsValid()) {
+Progress progress("Locating external symbol file",
+ prog_str.GetString().str());
+
// Has lldb already seen a module with this UUID?
+// Or have external lookup enabled in DebugSymbols on macOS.
clayborg wrote:
Should this go into `ModuleList::GetSharedModule()` or at some function that is
called by `ModuleList::GetSharedModule()`? If the
`ModuleList::GetSharedModule()` function is really quick, I hate to add really
spammy progress notifications that aren't useful. If there is code down in
`ModuleList::GetSharedModule()` that locates an external symbol file, then the
progress should go in that function that is known to take some amount of time,
but not here in a general area IMHO
https://github.com/llvm/llvm-project/pull/98845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/98845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] progressive progress reporting for darwin kernel/firmware (PR #98845)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/98845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add lldb version into initialize response lldb-dap (PR #98703)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/98703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Fix Android debugging (PR #98581)
clayborg wrote: We really need there to be some tests somehow or somewhere for this. Without tests we can't verify this doesn't get broken in the future. If we can't test with true android we could create a test platform layer that is only available in debug builds that could allow us to simulate things. https://github.com/llvm/llvm-project/pull/98581 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Fix remote executables load and caching (PR #98623)
clayborg wrote: Very nice. We really need to get an android test in for this, or at least a test that simulates what android does in some way to verify we don't regress this in the future. https://github.com/llvm/llvm-project/pull/98623 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)
@@ -1625,6 +1657,11 @@ void request_initialize(const llvm::json::Object
) {
"Get or set the repl behavior of lldb-dap evaluation requests.");
g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);
+ // singleStoppedEvent option is not from formal DAP specification. It is an
+ // lldb specific option to experiment stopped events behaivor against
+ // application with multiple threads.
+ g_dap.single_stopped_event =
+ GetBoolean(arguments, "singleStoppedEvent", false);
clayborg wrote:
Can we add this key/value pair as a launch configuration instead of adding it
from the IDE? Then we can document the feature in the package.json with lots of
warning text. Also the name should be lldb specific to ensure we don't conflict
with future VS code key/value pairs. Maybe add a "__lldbSingleStopEvent"?
There is also a type script plug-in in the upstream LLDB where you can add the
necessary setting to so if we leave this in the initialize packet, it can get
sent down here
https://github.com/llvm/llvm-project/pull/98568
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)
@@ -255,45 +253,75 @@ void SendThreadStoppedEvent() {
lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
uint32_t num_threads_with_reason = 0;
bool focus_thread_exists = false;
+ lldb::SBThread focus_thread, first_thread_with_reason;
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
const lldb::tid_t tid = thread.GetThreadID();
const bool has_reason = ThreadHasStopReason(thread);
// If the focus thread doesn't have a stop reason, clear the thread ID
if (tid == g_dap.focus_tid) {
+ focus_thread = thread;
focus_thread_exists = true;
if (!has_reason)
g_dap.focus_tid = LLDB_INVALID_THREAD_ID;
}
if (has_reason) {
++num_threads_with_reason;
- if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
+ if (first_tid_with_reason == LLDB_INVALID_THREAD_ID) {
first_tid_with_reason = tid;
+first_thread_with_reason = thread;
+ }
}
}
// We will have cleared g_dap.focus_tid if the focus thread doesn't have
// a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
// then set the focus thread to the first thread with a stop reason.
- if (!focus_thread_exists || g_dap.focus_tid == LLDB_INVALID_THREAD_ID)
+ if (!focus_thread_exists || g_dap.focus_tid == LLDB_INVALID_THREAD_ID) {
g_dap.focus_tid = first_tid_with_reason;
+focus_thread = first_thread_with_reason;
+ }
// If no threads stopped with a reason, then report the first one so
// we at least let the UI know we stopped.
if (num_threads_with_reason == 0) {
lldb::SBThread thread = process.GetThreadAtIndex(0);
g_dap.focus_tid = thread.GetThreadID();
g_dap.SendJSON(CreateThreadStopped(thread, stop_id));
+ } else if (g_dap.single_stopped_event) {
+// If single_stopped_event option is true only one stopped event will
+// be sent during debugger stop. The focused thread's stopped event is
+// preferred if it is stopped with a reason; otherwise, we simply use
+// the first stopped thread.
+//
+// This option would be preferred for VSCode IDE because multiple
+// stopped events would cause confusing UX.
+//
+// TODO: do we need to give priority to exception/signal stopped event
+// over normal stepping complete/breakpoint?
clayborg wrote:
This would be important
https://github.com/llvm/llvm-project/pull/98568
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)
clayborg wrote: This feature hides what other threads are doing when you stop and any other threads that stop for a reason can be hidden from the user. For example if we have 5 threads stop at a breakpoint, but we only report 1 for thread 1, do we see the right stop reason for the threads 2-5 in the UI? Can we tell they stopped at a breakpoint? Or we are pretending that they just stopped for no reason as far as the IDE is concerned? Do we need to type "thread list" in the debug console to see the real stop reasons? My main issue with this is lets say you set a breakpoint somewhere and this breakpoint is important and if you hit this breakpoint then you have found your bug. But you are stepping in another thread with this feature enabled. You will never know that this breakpoint was hit unless it eventually causes a fatal error. So it is easy to miss important breakpoints. That being said, Jeffrey's reason for this patch is the VS Code IDE doesn't do a great job at showing the current thread when you might be debugging some code that multiple threads are executing at the same time. You see a mess of thread PC indicators and the colors don't clearly tell you which one is your current thread and it becomes confusing for users. So I am not opposed to this feature as long as the description for the launch config option has a big warning in the description. https://github.com/llvm/llvm-project/pull/98568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add lldb version into initialize response lldb-dap (PR #98703)
@@ -1710,6 +1710,8 @@ void request_initialize(const llvm::json::Object
) {
body.try_emplace("supportsLogPoints", true);
// The debug adapter supports data watchpoints.
body.try_emplace("supportsDataBreakpoints", true);
+ // Putting version string. Note: this is not part of DAP spec.
+ body.try_emplace("version", g_dap.debugger.GetVersionString());
clayborg wrote:
I would add a dictionary named "__lldb" and the contents will be the "version":
like:
```
"__lldb": {
"version": ""
}
```
Then we can add more stuff in here in the future if we need to know things.
The Python version would be interesting as well possibly in the future.
https://github.com/llvm/llvm-project/pull/98703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add lldb version into initialize response lldb-dap (PR #98703)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/98703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add lldb version into initialize response lldb-dap (PR #98703)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/98703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
clayborg wrote: This did indeed break Android debugging. We reverted this in our repo and functionality was restored https://github.com/llvm/llvm-project/pull/96256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
clayborg wrote: We think this broke Android debugging on our end. We are investigating. https://github.com/llvm/llvm-project/pull/96256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix section printing to always align. (PR #98521)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/98521 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix section printing to always align. (PR #98521)
https://github.com/clayborg created https://github.com/llvm/llvm-project/pull/98521 Section IDs are 64 bit and if a section ID was over 4GB, then the tabular output of the "target modules dump sections" command would not align to the column headers. Also if the section type's name was too long, the output wouldn't algin. This patch fixes this issue. Old output looked like: ``` (lldb) image dump sections a.out Sections for '/tmp/a.out' (arm): SectID Type File Address Perm File Off. File Size Flags Section Name -- --- -- -- -- 0x container[0x1000-0x1010) rw- 0x0074 0x0010 0x a.out.PT_LOAD[0] 0x0001 data [0x1000-0x1010) rw- 0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data 0xfffe container[0x1000-0x1010) rw- 0x0084 0x 0x a.out.PT_TLS[0] 0x0002 zero-fill[0x1000-0x1010) rw- 0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss 0x0003 regular --- 0x0084 0x0001 0x a.out..strtab 0x0004 regular --- 0x0085 0x001f 0x a.out..shstrtab ``` New output looks like: ``` (lldb) image dump sections a.out Sections for '/tmp/a.out' (arm): SectID Type File Address Perm File Off. File Size Flags Section Name -- -- --- -- -- -- 0x container [0x1000-0x1010) rw- 0x0074 0x0010 0x a.out.PT_LOAD[0] 0x0001 data [0x1000-0x1010) rw- 0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data 0xfffe container [0x1000-0x1010) rw- 0x0084 0x 0x a.out.PT_TLS[0] 0x0002 zero-fill [0x1000-0x1010) rw- 0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss 0x0003 regular --- 0x0084 0x0001 0x a.out..strtab 0x0004 regular --- 0x0085 0x001f 0x a.out..shstrtab ``` >From ea7e58cf0677e6828b59038a527a4b23c189e548 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Thu, 11 Jul 2024 11:57:11 -0700 Subject: [PATCH] Fix section printing to always align. Section IDs are 64 bit and if a section ID was over 4GB, then the tabular output of the "target modules dump sections" command would not align to the column headers. Also if the section type's name was too long, the output wouldn't algin. This patch fixes this issue. Old output looked like: ``` (lldb) image dump sections a.out Sections for '/tmp/a.out' (arm): SectID Type File Address Perm File Off. File Size Flags Section Name -- --- -- -- -- 0x container[0x1000-0x1010) rw- 0x0074 0x0010 0x a.out.PT_LOAD[0] 0x0001 data [0x1000-0x1010) rw- 0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data 0xfffe container[0x1000-0x1010) rw- 0x0084 0x 0x a.out.PT_TLS[0] 0x0002 zero-fill[0x1000-0x1010) rw- 0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss 0x0003 regular --- 0x0084 0x0001 0x a.out..strtab 0x0004 regular --- 0x0085 0x001f 0x a.out..shstrtab ``` New output looks like: ``` (lldb) image dump sections a.out Sections for '/tmp/a.out' (arm): SectID Type File Address Perm File Off. File Size Flags Section Name -- -- --- -- -- -- 0x container [0x1000-0x1010) rw- 0x0074 0x0010 0x a.out.PT_LOAD[0] 0x0001 data [0x1000-0x1010) rw- 0x0074 0x0010
[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/98432 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)
https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/98432 >From ede6fc3939cbdf3a5744ff5e6919551a4b5dd438 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Wed, 10 Jul 2024 21:49:04 -0700 Subject: [PATCH 1/2] Fix a bug for PT_TLS segments getting loaded when they shouldn't. PT_LOAD and PT_TLS segments are top level sections in the ObjectFileELF section list. The two segments can often have the same program header p_vaddr and p_paddr values and this can cause section load list issues in LLDB if we load the PT_TLS segments. What happens is the SectionLoadList::m_addr_to_sect, when a library is loaded, will first map one of the sections named "PT_LOAD[0]" with the load address that matches the p_vaddr entry from the program header. Then the "PT_TLS[0]" would come along and try to load this section at the same address. This would cause the "PT_LOAD[0]" section to be unloaded as the SectionLoadList::m_addr_to_sect would replace the value for the matching p_vaddr with the last section to be seen. The sizes of the PT_TLS and PT_LOAD that have the same p_vaddr value don't need to have the same byte size, so this could cause lookups to fail for an addresses in the "PT_LOAD[0]" section or any of its children if the offset is greater than the offset size of the PT_TLS segment. It could also cause us to incorrectly attribute addresses from the "PT_LOAD[0]" to the "PT_TLS[0]" segment when doing lookups for offset that are less than the size of the PT_TLS segment. This fix stops us from loading PT_TLS segments in the section load lists and will prevent the bugs that resulted from this. No addresses the the DWARF refer to TLS data with a "file address" in any way. They all have TLS DWARF location expressions to locate these variables. We also don't have any support for having actual thread specific sections and having those sections resolve to something different for each thread, so there currently is no point in loading thread specific sections. Both the ObjectFileMachO and ObjectFileCOFF both ignore thread specific sections at the moment, so this brings the ObjectFileELF to parity with those plug-ins. I added a test into an existing test to verify that things work as expected. Prior to this fix with a real binary, the output of "target dump section-load-list" would look like this for the old LLDB: ``` // (lldb) target dump section-load-list // addr = 0x, section = 0x55d46ab8c510: 0xfffd container[0x-0x0628) r-- 0x 0x0628 0x a.out.PT_LOAD[0] // addr = 0x1000, section = 0x55d46ab8b0c0: 0xfffc container[0x1000-0x1185) r-x 0x1000 0x0185 0x a.out.PT_LOAD[1] // addr = 0x2000, section = 0x55d46ac040f0: 0xfffb container[0x2000-0x20cc) r-- 0x2000 0x00cc 0x a.out.PT_LOAD[2] // addr = 0x3db0, section = 0x55d46ab7cef0: 0xfff6 container[0x3db0-0x3db4) r-- 0x2db0 0x 0x a.out.PT_TLS[0] ``` And this for the fixed LLDB: ``` // (lldb) target dump section-load-list // addr = 0x, section = 0x105f0a9a8: 0xfffd container[0x-0x0628) r-- 0x 0x0628 0x a.out.PT_LOAD[0] // addr = 0x1000, section = 0x105f0adb8: 0xfffc container[0x1000-0x1185) r-x 0x1000 0x0185 0x a.out.PT_LOAD[1] // addr = 0x2000, section = 0x105f0af48: 0xfffb container[0x2000-0x20cc) r-- 0x2000 0x00cc 0x a.out.PT_LOAD[2] // addr = 0x3db0, section = 0x105f0b078: 0xfffa container[0x3db0-0x4028) rw- 0x2db0 0x0274 0x a.out.PT_LOAD[3] ``` We can see that previously the "PT_LOAD[3]" segment would be removed from the section load list, and after the fix it remains and there is on PT_TLS in the loaded sections. --- .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 14 +++ .../ELF/PT_TLS-overlap-PT_LOAD.yaml | 25 ++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp index 5c6b475044be5..51bd34e95c77d 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -717,6 +717,20 @@ bool ObjectFileELF::SetLoadAddress(Target , lldb::addr_t value, // Iterate through the object file sections to find all of the sections // that have SHF_ALLOC in their flag bits. SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx)); + +// PT_TLS segments can have
[Lldb-commits] [lldb] [LLDB][SaveCore] Add SBCoreDumpOptions Object, and SBProcess::SaveCore() overload (PR #98403)
@@ -0,0 +1,67 @@
+//===-- SBCoreDumpOptions.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBCoreDumpOptions.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Symbol/CoreDumpOptions.h"
+#include "lldb/Utility/Instrumentation.h"
+
+#include "Utils.h"
+
+using namespace lldb;
+
+SBCoreDumpOptions::SBCoreDumpOptions(const char *filePath) {
+ LLDB_INSTRUMENT_VA(this, filePath);
+ lldb_private::FileSpec fspec(filePath);
+ lldb_private::FileSystem::Instance().Resolve(fspec);
+ m_opaque_up = std::make_unique(fspec);
+}
+
+SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+const SBCoreDumpOptions &
+SBCoreDumpOptions::operator=(const SBCoreDumpOptions ) {
+ LLDB_INSTRUMENT_VA(this, rhs);
+
+ if (this != )
+m_opaque_up = clone(rhs.m_opaque_up);
+ return *this;
+}
+
+void SBCoreDumpOptions::SetCoreDumpPluginName(const char *name) {
+ m_opaque_up->SetCoreDumpPluginName(name);
+}
+
+void SBCoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) {
+ m_opaque_up->SetCoreDumpStyle(style);
+}
+
+const std::optional
+SBCoreDumpOptions::GetCoreDumpPluginName() const {
+ const auto = m_opaque_up->GetCoreDumpPluginName();
+ if (name->empty())
+return std::nullopt;
+ return name->data();
+}
+
+const char *SBCoreDumpOptions::GetOutputFile() const {
+ return m_opaque_up->GetOutputFile().GetFilename().AsCString();
+}
clayborg wrote:
Return a SBFileSpec object here. Also remove the "const" as it doesn't mean
anything because it just means you can't change the `m_opaque_up` value, but it
will still allow you to call a non const function on the pointer contained in
`m_opaque_up`... You will need to add the `SBCoreDumpOptions` as friend class
in SBFileSpec.h so that you can call the constructor that uses a
`lldb_private::FileSpec`. Then this code becomes:
```
lldb::SBFileSpec SBCoreDumpOptions::GetOutputFile() {
return lldb::SBFileSpec(m_opaque_up->GetOutputFile());
}
```
https://github.com/llvm/llvm-project/pull/98403
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
