jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, bulbazord.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
This adds a new LC_NOTE to ObjectFileMachO to save the thread IDs of threads,
and restore them in ProcessMachCore when reading the corefile back. The
LC_NOTE payload format is
struct thread_extrainfo {
uint32_t version; // currently 1
uint32_t elem_size; // size of each array element
struct {
uint64_t tid; // tid override for each LC_THREAD, 0 means none
provided
} __attribute__((packed)) thread_extras[NUM_LC_THREADS];
};
My intention is that we can add additional fields in the future without bumping
the version number; existing fields must always be included. Each field will
have sentinel values to indicate that they are unspecified -- if we add a
"thread priority" uint64_t next year, but a producer only wants to specify
thread priorities and not thread IDs, they can put 0 values for the thread IDs.
Consumers can skip fields that they don't know about.
The one down side to this layout is that variable-length fields can't be
incorporated well -- thread names, libdispatch queue names on macOS for
instance. It might be better to have each element specify its size, so we can
have variable length entries. I'd love to hear opinions, the more I think
about it, the more I worry someone is going to request thread names in
corefiles about a week after I land this.
The patch adds support to ObjectFileMachO to emit this new LC_NOTE when
creating a corefile for `process save-core`, and support to
ObjectFileMachO/ProcessMachCore/ThreadMachCore to read those values back in
again. It adds a check to an existing API test that has sixteen threads for
confirming that we select the faulting thread in a corefile.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158785
Files:
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
lldb/source/Plugins/Process/mach-core/ThreadMachCore.h
lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
Index: lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
===================================================================
--- lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
+++ lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
@@ -26,6 +26,11 @@
self.runCmd("continue")
self.runCmd("process save-core -s stack " + corefile)
+ live_tids = []
+ if self.TraceOn():
+ self.runCmd("thread list")
+ for t in process.threads:
+ live_tids.append(t.GetThreadID())
process.Kill()
self.dbg.DeleteTarget(target)
@@ -42,3 +47,11 @@
self.assertEqual(
thread.GetStopDescription(256), "ESR_EC_DABORT_EL0 (fault address: 0x0)"
)
+
+ if self.TraceOn():
+ self.runCmd("thread list")
+ i = 0
+ while i < process.GetNumThreads():
+ t = process.GetThreadAtIndex(i)
+ self.assertEqual(t.GetThreadID(), live_tids[i])
+ i = i + 1
Index: lldb/source/Plugins/Process/mach-core/ThreadMachCore.h
===================================================================
--- lldb/source/Plugins/Process/mach-core/ThreadMachCore.h
+++ lldb/source/Plugins/Process/mach-core/ThreadMachCore.h
@@ -17,7 +17,8 @@
class ThreadMachCore : public lldb_private::Thread {
public:
- ThreadMachCore(lldb_private::Process &process, lldb::tid_t tid);
+ ThreadMachCore(lldb_private::Process &process, lldb::tid_t tid,
+ uint32_t objfile_lc_thread_idx);
~ThreadMachCore() override;
@@ -57,6 +58,7 @@
std::string m_dispatch_queue_name;
lldb::addr_t m_thread_dispatch_qaddr;
lldb::RegisterContextSP m_thread_reg_ctx_sp;
+ uint32_t m_objfile_lc_thread_idx;
// Protected member functions.
bool CalculateStopInfo() override;
Index: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
@@ -33,9 +33,11 @@
// Thread Registers
-ThreadMachCore::ThreadMachCore(Process &process, lldb::tid_t tid)
+ThreadMachCore::ThreadMachCore(Process &process, lldb::tid_t tid,
+ uint32_t objfile_lc_thread_idx)
: Thread(process, tid), m_thread_name(), m_dispatch_queue_name(),
- m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS), m_thread_reg_ctx_sp() {}
+ m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS), m_thread_reg_ctx_sp(),
+ m_objfile_lc_thread_idx(objfile_lc_thread_idx) {}
ThreadMachCore::~ThreadMachCore() { DestroyThread(); }
@@ -81,8 +83,8 @@
ObjectFile *core_objfile =
static_cast<ProcessMachCore *>(process_sp.get())->GetCoreObjectFile();
if (core_objfile)
- m_thread_reg_ctx_sp =
- core_objfile->GetThreadContextAtIndex(GetID(), *this);
+ m_thread_reg_ctx_sp = core_objfile->GetThreadContextAtIndex(
+ m_objfile_lc_thread_idx, *this);
}
reg_ctx_sp = m_thread_reg_ctx_sp;
} else {
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -594,9 +594,38 @@
ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
if (core_objfile) {
+ std::set<tid_t> used_tids;
const uint32_t num_threads = core_objfile->GetNumThreadContexts();
- for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
- ThreadSP thread_sp(new ThreadMachCore(*this, tid));
+ std::vector<tid_t> tids;
+ if (core_objfile->GetCorefileThreadExtraInfos(tids)) {
+ assert(tids.size() == num_threads);
+
+ // populate used_tids
+ for (uint32_t i = 0; i < num_threads; i++) {
+ if (tids[i] != LLDB_INVALID_THREAD_ID)
+ used_tids.insert(tids[i]);
+ }
+ // If any threads have an unspecified thread id,
+ // find an unused number, use it instead.
+ tid_t current_unused_tid = 0;
+ for (uint32_t i = 0; i < num_threads; i++) {
+ if (tids[i] == LLDB_INVALID_THREAD_ID) {
+ while (used_tids.find(current_unused_tid) != used_tids.end()) {
+ current_unused_tid++;
+ }
+ tids[i] = current_unused_tid;
+ used_tids.insert(current_unused_tid);
+ }
+ }
+ } else {
+ // No metadata, insert numbers sequentially from 0.
+ for (uint32_t i = 0; i < num_threads; i++) {
+ tids.push_back(i);
+ }
+ }
+
+ for (uint32_t i = 0; i < num_threads; i++) {
+ ThreadSP thread_sp(new ThreadMachCore(*this, tids[i], i));
new_thread_list.AddThread(thread_sp);
}
}
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -128,6 +128,8 @@
lldb_private::UUID &uuid,
ObjectFile::BinaryType &type) override;
+ bool GetCorefileThreadExtraInfos(std::vector<lldb::tid_t> &tids) override;
+
bool LoadCoreFileImages(lldb_private::Process &process) override;
lldb::RegisterContextSP
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -70,6 +70,7 @@
#include <bitset>
#include <memory>
#include <optional>
+#include <sstream>
// Unfortunately the signpost header pulls in the system MachO header, too.
#ifdef CPU_TYPE_ARM
@@ -5669,6 +5670,79 @@
return false;
}
+bool ObjectFileMachO::GetCorefileThreadExtraInfos(std::vector<tid_t> &tids) {
+ tids.clear();
+ Log *log(GetLog(LLDBLog::Object | LLDBLog::Process | LLDBLog::Thread));
+ ModuleSP module_sp(GetModule());
+ if (!module_sp)
+ return false;
+
+ std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+ offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+ for (uint32_t i = 0; i < m_header.ncmds; ++i) {
+ const uint32_t cmd_offset = offset;
+ llvm::MachO::load_command lc = {};
+ if (m_data.GetU32(&offset, &lc.cmd, 2) == nullptr)
+ break;
+ if (lc.cmd == LC_NOTE) {
+ char data_owner[17];
+ memset(data_owner, 0, sizeof(data_owner));
+ m_data.CopyData(offset, 16, data_owner);
+ offset += 16;
+ uint64_t fileoff = m_data.GetU64_unchecked(&offset);
+ uint64_t size = m_data.GetU64_unchecked(&offset);
+
+ if (strcmp("thread extrainfo", data_owner) == 0 && size >= 8) {
+ offset = fileoff;
+ uint32_t version;
+ if (!m_data.GetU32(&offset, &version, 1) || version != 1) {
+ LLDB_LOGF(log,
+ "Unable to read 'thread extrainfo' LC_NOTE version, or "
+ "version %d is not handled",
+ version);
+ return false;
+ }
+ uint32_t elem_size;
+ if (!m_data.GetU32(&offset, &elem_size, 1)) {
+ LLDB_LOGF(log, "Unable to read 'thread extrainfo' LC_NOTE version, "
+ "could not read element size");
+ return false;
+ }
+ const uint32_t num_threads = GetNumThreadContexts();
+ for (uint32_t i = 0; i < num_threads; i++) {
+ tid_t tid;
+ offset_t element_offset = offset;
+ if (!m_data.GetU64(&offset, &tid, 1)) {
+ LLDB_LOGF(log,
+ "Unable to read tid %u from 'thread extrainfo' LC_NOTE",
+ i);
+ return false;
+ }
+ if (tid == 0)
+ tid = LLDB_INVALID_THREAD_ID;
+ tids.push_back(tid);
+ offset = element_offset + elem_size;
+ }
+ if (log) {
+ std::stringstream logmsg;
+ logmsg << "LC_NOTE 'thread extrainfo' found, version " << version;
+ logmsg << " elem_size " << elem_size << ": ";
+ for (tid_t tid : tids) {
+ if (tid == LLDB_INVALID_THREAD_ID)
+ logmsg << " LLDB_INVALID_THREAD_ID";
+ else
+ logmsg << "0x" << std::hex << tid;
+ }
+ LLDB_LOGF(log, "%s", logmsg.str().c_str());
+ }
+ return true;
+ }
+ }
+ offset = cmd_offset + lc.cmdsize;
+ }
+ return false;
+}
+
lldb::RegisterContextSP
ObjectFileMachO::GetThreadContextAtIndex(uint32_t idx,
lldb_private::Thread &thread) {
@@ -6652,6 +6726,10 @@
mach_header.sizeofcmds += sizeof(llvm::MachO::note_command);
}
+ // LC_NOTE "thread extrainfo"
+ mach_header.ncmds++;
+ mach_header.sizeofcmds += sizeof(llvm::MachO::note_command);
+
// LC_NOTE "all image infos"
mach_header.ncmds++;
mach_header.sizeofcmds += sizeof(llvm::MachO::note_command);
@@ -6694,6 +6772,22 @@
lc_notes.push_back(std::move(addrable_bits_lcnote_up));
}
+ // Add "thread extrainfo" LC_NOTE
+ std::unique_ptr<LCNoteEntry> thread_extrainfo_lcnote_up(
+ new LCNoteEntry(addr_byte_size, byte_order));
+ thread_extrainfo_lcnote_up->name = "thread extrainfo";
+ thread_extrainfo_lcnote_up->payload_file_offset = file_offset;
+ thread_extrainfo_lcnote_up->payload.PutHex32(1); // version
+ thread_extrainfo_lcnote_up->payload.PutHex32(8); // elem_size
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ thread_extrainfo_lcnote_up->payload.PutHex64(
+ thread_sp->GetID()); // tid
+ }
+ file_offset += thread_extrainfo_lcnote_up->payload.GetSize();
+ file_offset = llvm::alignTo(file_offset, 16);
+ lc_notes.push_back(std::move(thread_extrainfo_lcnote_up));
+
// Add "all image infos" LC_NOTE
std::unique_ptr<LCNoteEntry> all_image_infos_lcnote_up(
new LCNoteEntry(addr_byte_size, byte_order));
@@ -6848,8 +6942,8 @@
ObjectFileMachO::MachOCorefileAllImageInfos
ObjectFileMachO::GetCorefileAllImageInfos() {
MachOCorefileAllImageInfos image_infos;
- Log *log(
- GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
+ Log *log(GetLog(LLDBLog::Object | LLDBLog::Symbols | LLDBLog::Process |
+ LLDBLog::DynamicLoader));
// Look for an "all image infos" LC_NOTE.
lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
@@ -6957,7 +7051,7 @@
bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
MachOCorefileAllImageInfos image_infos = GetCorefileAllImageInfos();
- Log *log = GetLog(LLDBLog::DynamicLoader);
+ Log *log = GetLog(LLDBLog::Object | LLDBLog::DynamicLoader);
Status error;
bool found_platform_binary = false;
Index: lldb/include/lldb/Symbol/ObjectFile.h
===================================================================
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -538,6 +538,30 @@
return false;
}
+ /// Get metadata about threads from the corefile.
+ ///
+ /// The corefile may have metadata (e.g. a Mach-O "thread extrainfo"
+ /// LC_NOTE) which for the threads in the process; this method tries
+ /// to retrieve them.
+ ///
+ /// \param[out] tids
+ /// Filled in with a vector of tid_t's that matches the number
+ /// of threads in the corefile (ObjectFile::GetNumThreadContexts).
+ /// If a tid is not specified for one of the corefile threads,
+ /// that entry in the vector will have LLDB_INVALID_THREAD_ID and
+ /// the caller should assign a tid to the thread that does not
+ /// conflict with the ones provided in this array.
+ /// As additional metadata are added, this method may return a
+ /// \a tids vector with no thread id's specified at all; the
+ /// corefile may only specify one of the other metadata.
+ ///
+ /// \return
+ /// Returns true if thread metadata was found in this corefile.
+ ///
+ virtual bool GetCorefileThreadExtraInfos(std::vector<lldb::tid_t> &tids) {
+ return false;
+ }
+
virtual lldb::RegisterContextSP
GetThreadContextAtIndex(uint32_t idx, lldb_private::Thread &thread) {
return lldb::RegisterContextSP();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
