jasonmolenda updated this revision to Diff 553324.
jasonmolenda added a subscriber: jingham.
jasonmolenda added a comment.
After thinking about this more, and talking with @jingham I rewrote the patch
so LC_NOTE "thread extrainfo" is a JSON dictionary with a key `threads` that
has an array. The number of entries in the array must match the number of
LC_THREADs in the Mach-O corefile. Each array entry may have a `thread_id` key
with a thread id for that LC_THREAD, or lldb will create a thread_id for it. I
expect we will add more per-thread keys in the future.
The payload looks like
{"threads":[{"thread_id":18368681},{"thread_id":18368703},{"thread_id":18368704}]}
and JSON is stored as a c-string, requires a nul byte '\0' at the end.
I've treated the LC_NOTEs in Mach-O as strictly defined binary data until now,
so producers and consumers both had a reference to work with. But I think in
the case of per-thread information in a corefile, we will have different people
wanting to add different fields -- some fixed width, some variable length --
and specifying a JSON format that can be extended more easily is probably the
correct way to go here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158785/new/
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,87 @@
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) {
+ offset = fileoff;
+ const char *payload = m_data.GetCStr(&offset, size);
+ if (!payload) {
+ LLDB_LOGF(log,
+ "Unable to read 'thread extrainfo' LC_NOTE JSON contents "
+ "size %" PRIu64,
+ size);
+ return false;
+ }
+ StructuredData::ObjectSP object_sp = StructuredData::ParseJSON(payload);
+ StructuredData::Dictionary *dict = object_sp->GetAsDictionary();
+ if (!dict) {
+ LLDB_LOGF(log, "Unable to read 'thread extrainfo' LC_NOTE, did not "
+ "get a dictionary.");
+ return false;
+ }
+ StructuredData::Array *threads;
+ if (!dict->GetValueForKeyAsArray("threads", threads) || !threads) {
+ LLDB_LOGF(log, "Unable to read 'thread extrainfo' LC_NOTE, could not "
+ "find threads.");
+ return false;
+ }
+ if (threads->GetSize() != GetNumThreadContexts()) {
+ LLDB_LOGF(log, "Unable to read 'thread extrainfo' LC_NOTE, number of "
+ "threads does not match number of LC_THREADS.");
+ return false;
+ }
+ const uint32_t num_threads = threads->GetSize();
+ for (uint32_t i = 0; i < num_threads; i++) {
+ StructuredData::Dictionary *thread;
+ if (!threads->GetItemAtIndexAsDictionary(i, thread) || !thread) {
+ LLDB_LOGF(log,
+ "Unable to read 'thread extrainfo' LC_NOTE, threads "
+ "array does not have a dictionary at index %u.",
+ i);
+ return false;
+ }
+ tid_t tid = LLDB_INVALID_THREAD_ID;
+ if (thread->GetValueForKeyAsInteger<tid_t>("thread_id", tid))
+ if (tid == 0)
+ tid = LLDB_INVALID_THREAD_ID;
+ tids.push_back(tid);
+ }
+
+ if (log) {
+ StreamString logmsg;
+ logmsg.Printf("LC_NOTE 'thread extrainfo' found: ");
+ dict->Dump(logmsg, /* pretty_print */ false);
+ LLDB_LOGF(log, "%s", logmsg.GetData());
+ }
+ return true;
+ }
+ }
+ offset = cmd_offset + lc.cmdsize;
+ }
+ return false;
+}
+
lldb::RegisterContextSP
ObjectFileMachO::GetThreadContextAtIndex(uint32_t idx,
lldb_private::Thread &thread) {
@@ -6652,6 +6734,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 +6780,32 @@
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;
+
+ StructuredData::DictionarySP dict(
+ std::make_shared<StructuredData::Dictionary>());
+ StructuredData::ArraySP threads(
+ std::make_shared<StructuredData::Array>());
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ StructuredData::DictionarySP thread(
+ std::make_shared<StructuredData::Dictionary>());
+ thread->AddIntegerItem("thread_id", thread_sp->GetID());
+ threads->AddItem(thread);
+ }
+ dict->AddItem("threads", threads);
+ StreamString strm;
+ dict->Dump(strm);
+ thread_extrainfo_lcnote_up->payload.PutCString(strm.GetData());
+
+ 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 +6960,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 +7069,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
