This is an archive of the discontinued LLVM Phabricator instance.

Add ability for lldb to load binaries in a process/corefile given only a load address & no dynamic loader
ClosedPublic

Authored by jasonmolenda on Jul 29 2022, 6:30 PM.

Details

Summary

I'm adding support for a firmware type environment, without any dynamic linker, where we may have more than one binary in memory and we'll only be given the load addresses of the binaries.

lldb already has support for loading a single binary given a UUID & address or slide. It slowly grew to the point (my fault) where three different classes (ProcessGDBRemote, ProcessMachCore, ObjectFileMachO) had versions of code to find and load the correct binary at the correct address into the Target. This would have been yet another variation added to all three places, so obviously the first thing to do was make a unified utility function to do this. I chose a static method in the DynamicLoader base class, LoadBinaryWithUUIDAndAddress() which handles this new case, as well as most of the others. I removed the other copies of code that were doing similar things from those three classes.

I thought of making this a static method in DynamicLoaderStatic. I couldn't come up with a strong preference either way -- it's only a method you'll be calling when you don't have a DynamicLoader plugin, so maybe that is the best place for it.

I added a new key-value to qProcessInfo, binary-addresses, which is a base16 array of addresses where binaries are. When given no UUID, lldb will try to read the binary from memory and find the UUID in the in-memory load commands, then do the normal "find me a binary for this UUID" schemes, loading it into the Target.

lldb already has "main bin spec" and "load binary" LC_NOTEs that always needed a UUID until today; this code now handles the case where no UUID is supplied, but we have a load address. I was a little unhappy to find that UUID::IsValid() returns true for an all-zeroes UUID which seems like it should probably return invalid? The LC_NOTEs use all-zeroes to indicate "no uuid", whereas our UUID class uses "optional data exists". I have a few checks in ObjectFileMachO where I check the uuid_t to see if it is all-zeros before I copy them in to the UUID object.

I tested the qProcessInfo load method manually with a hacked debugserver & lldb which had no DynamicLoaderMacOS plugin; anything less than that looks like a valid macOS debug session and it was difficult to fool lldb into using these correctly.

I wrote a fun test case for the corefile support where it builds three test binaries - an executable and two shared libraries - and a program that creates a corefile of those three binaries at their load addresses. Then we can run lldb on the corefile and confirm it loads binaries already known to lldb, via dsymForUUID, or loads a memory image as a last resort. It took a bit to get the corefile creator working correctly, but it's pretty cool.

One tiny oddity I noticed is that we've ended up with

include/lldb/Target/DynamicLoader.h
source/Core/DynamicLoader.cpp

I didn't look into which one was intended to be correct & fix it for now.

Diff Detail

Event Timeline

jasonmolenda created this revision.Jul 29 2022, 6:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2022, 6:30 PM
jasonmolenda requested review of this revision.Jul 29 2022, 6:30 PM

I was using an obsoleted linker flag to seg the vmaddrs of two dylibs and laying them out there in the corefile. This won't work long-term, and I really wanted to make sure lldb will slide binaries correctly, so I added a new feature to my corefile creator which will apply a slide to the binaries it puts in the corefiles, modifying the segment/section load commands with the slid vmaddrs as if dyld had put them there.

JDevlieghere accepted this revision.Aug 2 2022, 9:02 AM

A few minor nits but the change looks sound to me. It's great to see the existing code getting unified and nice job on that test. LGTM.

lldb/source/Core/DynamicLoader.cpp
192–194

If it wasn't for the fixed-length buffer I probably would've ignored it, but you have the same code on line 222. Might be worth extracting this into a little static helper function/lambda. Something like:

BufferSP ReadModuleFromMemory(ProcessSP process, lldb::addr_t addr) {
    char namebuf[80];
    snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
    return process->ReadModuleFromMemory(FileSpec(namebuf), addr);
}
238–239

Use LLDB_LOGF which includes the if(log) check. There's some more instances of that below.

lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
2

Are there other tests that can (or will) benefit from this utility? If so I think it would make sense to make it a proper tool that we build with CMake (similar to lldb-test and lldb-instr). That's more of a suggestion for the future, as is I think it's totally fine for this to be part of the test.

This revision is now accepted and ready to land.Aug 2 2022, 9:02 AM

Update patch to address Jonas' feedback, add a better descriptive comment on what my custom corefile creator in the test case does.