Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

Authored by jasonmolenda on Aug 10 2023, 3:05 PM.



This patch addresses two issues I found while looking into a problem yesterday. First, SBTarget::AddModule calls Target::GetOrCreate which will find the module in the global module list, the current target, and will call the DebugSymbols framework on macOS and look in specified directories, and do a Spotlight search of the local computer. But it doesn't call Symbols::DownloadObjectAndSymbolFile() with a "force expensive lookup" which we should assume the program is trying to do if they gave us a UUID to look up. This matches the behavior of target modules add -u .... today in the lldb command line interface.

Also, if the SB API user has created a Target which has no architecture, we normally will set the Target's arch as soon as we find a hint about the arch -- e.g. when we connect to a gdb remote serial protocol stub, or when we get a corefile. But in this case, they are creating a Target with no arch, adding a module to it. We should use the same scheme where a Target inherits the first likely ArchSpec we see when it has none of its own.

Add a test for these two things, using a fake shell (which Jonas has pointed out in the past, I really need to create a utility function that creates these given an array of uuid/binary/dsyms, which I agree with but haven't done yet. :/)

Diff Detail

Event Timeline

jasonmolenda created this revision.Aug 10 2023, 3:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2023, 3:05 PM
jasonmolenda requested review of this revision.Aug 10 2023, 3:05 PM

One more bit of explanation about the changes: There is an SBTarget::AddModule which takes individual parts of a module spec, and an SBTarget::AddModule which takes an SBModuleSpec. I was going to need to duplicate my code to force the expensive search & update the Target's arch if it didn't have one in both, so I had the first method create an SBModuleSpec with its information and call the second.

The idea seems fine to me. A few nits and comments, otherwise LGTM.


nit: add a comment next to true to indicate that it's for copy_executable


nit: It's redundant to check sb_module.GetSP() because sb_module.IsValid() already does that.


I know this is already done in a few places but we should really use llvm-dwarfdump so this can be a more portable test...

jasonmolenda marked 2 inline comments as done.Aug 11 2023, 1:08 PM
jasonmolenda added inline comments.

Yeah Jonas made that point last week about another test I was getting the uuid from a file. I don't know how to refer to something in the build products directory in an API test like this, is that what you meant? It's not installed on a standard macOS system in any path.

Incorporate Alex's suggestions, not sure about using llvm-dwarfdump yet tho.

bulbazord added inline comments.Aug 11 2023, 1:36 PM

Yeah I was referring to the build product llvm-dwarfdump. I did a quick search through the test suite but it doesn't look like we do this anywhere right now, so we'll need to figure out how to do it first.

This revision is now accepted and ready to land.Aug 11 2023, 1:36 PM