Skip to content

[lldb] Fix loading UUIDs from ELF headers. #117028

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Conversation

clayborg
Copy link
Collaborator

@clayborg clayborg commented Nov 20, 2024

A previous patch added the ability to load UUID from ELF headers using the program header and finding PT_NOTE entries. The fix would attempt to read the data for the PT_NOTE from memory, but it didn't slide the address so it ended up only working for the main executable if it wasn't moved in memory. This patch slides the address and adds logging.

All processes map the ELF header + program headers + some program header contents into memory. The program header for the PT_NOTE entries are mapped, but the p_vaddr doesn't get relocated and is relative to the load address of the ELF header. So we take a "p_vaddr" (file address) and convert it into a load address in the process so we can load the correct bytes that contain the PT_NOTE contents.

A previous patch added the ability to load UUID from ELF headers using the program header and finding PT_NOTE entries. The fix would attempt to read the data for the PT_NOTE from memory, but it didn't slide the address so it ended up only working for the main executable if it wasn't moved in memory. This patch slides the address and adds logging.
@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

A previous patch added the ability to load UUID from ELF headers using the program header and finding PT_NOTE entries. The fix would attempt to read the data for the PT_NOTE from memory, but it didn't slide the address so it ended up only working for the main executable if it wasn't moved in memory. This patch slides the address and adds logging.


Full diff: https://github.com/llvm/llvm-project/pull/117028.diff

1 Files Affected:

  • (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+15-5)
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 7955594bf5d94c..e58c06b9bd333c 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -224,7 +224,7 @@ Status ProcessElfCore::DoLoadCore() {
   ArchSpec core_arch(m_core_module_sp->GetArchitecture());
   target_arch.MergeFrom(core_arch);
   GetTarget().SetArchitecture(target_arch);
- 
+
   SetUnixSignals(UnixSignals::Create(GetArchitecture()));
 
   // Ensure we found at least one thread that was stopped on a signal.
@@ -276,8 +276,13 @@ Status ProcessElfCore::DoLoadCore() {
 }
 
 void ProcessElfCore::UpdateBuildIdForNTFileEntries() {
+  Log *log = GetLog(LLDBLog::Process);
   for (NT_FILE_Entry &entry : m_nt_file_entries) {
     entry.uuid = FindBuidIdInCoreMemory(entry.start);
+    if (log && entry.uuid.IsValid())
+      LLDB_LOGF(log, "%s found UUID @ %16.16" PRIx64 ": %s \"%s\"",
+                __FUNCTION__, entry.start, entry.uuid.GetAsString().c_str(),
+                entry.path.c_str());
   }
 }
 
@@ -875,7 +880,7 @@ llvm::Error ProcessElfCore::parseOpenBSDNotes(llvm::ArrayRef<CoreNote> notes) {
 /// - NT_SIGINFO - Information about the signal that terminated the process
 /// - NT_AUXV - Process auxiliary vector
 /// - NT_FILE - Files mapped into memory
-/// 
+///
 /// Additionally, for each thread in the process the core file will contain at
 /// least the NT_PRSTATUS note, containing the thread id and general purpose
 /// registers. It may include additional notes for other register sets (floating
@@ -1034,15 +1039,20 @@ UUID ProcessElfCore::FindBuidIdInCoreMemory(lldb::addr_t address) {
     std::vector<uint8_t> note_bytes;
     note_bytes.resize(program_header.p_memsz);
 
-    byte_read = ReadMemory(program_header.p_vaddr, note_bytes.data(),
-                           program_header.p_memsz, error);
+    // We need to slide the address of the p_vaddr as these values don't get
+    // relocated in memory.
+    const lldb::addr_t vaddr = program_header.p_vaddr + address;
+    byte_read = ReadMemory(vaddr, note_bytes.data(), program_header.p_memsz,
+                           error);
     if (byte_read != program_header.p_memsz)
       continue;
     DataExtractor segment_data(note_bytes.data(), note_bytes.size(),
                                GetByteOrder(), addr_size);
     auto notes_or_error = parseSegment(segment_data);
-    if (!notes_or_error)
+    if (!notes_or_error) {
+      llvm::consumeError(notes_or_error.takeError());
       return invalid_uuid;
+    }
     for (const CoreNote &note : *notes_or_error) {
       if (note.info.n_namesz == 4 &&
           note.info.n_type == llvm::ELF::NT_GNU_BUILD_ID &&

Copy link

github-actions bot commented Nov 20, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

I'm a bit confused as to how we have a core file but also things are moved in memory.

Can you give an explanation of how this occurs? Please add it to the PR description.

Also, tests? (which is partly why I ask how this occurs)

@@ -224,7 +224,7 @@ Status ProcessElfCore::DoLoadCore() {
ArchSpec core_arch(m_core_module_sp->GetArchitecture());
target_arch.MergeFrom(core_arch);
GetTarget().SetArchitecture(target_arch);

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would push these whitespace changes directly, to keep this change neat.

@DavidSpickett DavidSpickett changed the title Fix loading UUIDs from ELF headers. [lldb] Fix loading UUIDs from ELF headers. Nov 21, 2024
@clayborg
Copy link
Collaborator Author

I'm a bit confused as to how we have a core file but also things are moved in memory.

Can you give an explanation of how this occurs? Please add it to the PR description.

Will do.

Also, tests? (which is partly why I ask how this occurs)

Tests are hard to create as we don't have a custom way to create ELF core files and obj2yaml and yaml2obj don't work for ELF files that have program header data only. Core files contain program headers with data only in the file, so we can't take an existing core file and obj2yaml it and then remove parts of it because the resulting file will be missing all of the program header data. If anyone has a solution as to how to hand craft a ELF core file using YAML please let me know and I will make another PR to test this.

@clayborg clayborg merged commit bcf654c into llvm:main Nov 21, 2024
7 checks passed
@clayborg clayborg deleted the fix-elf-uuids branch November 21, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants