Page MenuHomePhabricator

[Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime
ClosedPublic

Authored by xiaobai on Jun 2 2019, 12:41 PM.

Details

Summary

LookupRuntimeSymbol seems like a general LanguageRuntime method.
Although no other language runtime currently implements this, there's no
reason another language runtime couldn't use this.

Additionally, this breaks IRExecutionUnit's dependency on
ObjCLanguageRuntime.

Diff Detail

Repository
rL LLVM

Event Timeline

xiaobai created this revision.Jun 2 2019, 12:41 PM
labath added a comment.Jun 3 2019, 1:18 AM

If this is supposed to be truly language-agnostic, shouldn't we be iterating over all language plugins and asking all of them for the "runtime symbol" (not that I know what a runtime symbol is, really)?

Otherwise, this is still language-specific behavior, even though it's not visible to the linker because it's hidden by an enum and a virtual interface.

If this is supposed to be truly language-agnostic, shouldn't we be iterating over all language plugins and asking all of them for the "runtime symbol" (not that I know what a runtime symbol is, really)?

Otherwise, this is still language-specific behavior, even though it's not visible to the linker because it's hidden by an enum and a virtual interface.

Mmm, yes, you're right, that should be happening. The function is FindInRuntimes, not FindInObjCRuntime after all. Thanks for pointing that out, let me change that.

xiaobai updated this revision to Diff 202763.Jun 3 2019, 11:34 AM

Make the behavior in IRExecutionUnit actually language agnostic.

Also, reverse the order of the for loops.

clayborg added inline comments.
include/lldb/Target/ObjCLanguageRuntime.h
270–272 ↗(On Diff #202763)

Which language has this filled in? Only Swift?

xiaobai marked an inline comment as done.Jun 3 2019, 2:45 PM
xiaobai added inline comments.
include/lldb/Target/ObjCLanguageRuntime.h
270–272 ↗(On Diff #202763)

Objective-C is the only thing that has this filled in at the moment, as I understand it.

clayborg accepted this revision.Jun 3 2019, 2:52 PM
clayborg added inline comments.
include/lldb/Target/ObjCLanguageRuntime.h
270–272 ↗(On Diff #202763)

There must be a V1 or V2 implementation somewhere? From the looks of things here, it seems like we are pulling it out, but that must not be what is happening. So the V1 and V2 impls must have this already set as "lldb::addr_t LookupRuntimeSymbol(ConstString name) override;"?

This revision is now accepted and ready to land.Jun 3 2019, 2:52 PM
xiaobai marked an inline comment as done.Jun 3 2019, 2:58 PM
xiaobai added inline comments.
include/lldb/Target/ObjCLanguageRuntime.h
270–272 ↗(On Diff #202763)

Yes, AppleObjCRuntimeV2 has an override for this method.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2019, 3:43 PM
jingham added a subscriber: jingham.Jun 3 2019, 6:35 PM

This looks fine to me.

For context, ObjC has symbols that point into the runtime that tell you things like the current offsets of the members of an ObjC class. In debug builds the symbols are present, but the runtime doesn't depend on the symbols per se, since it reads the data directly from the runtime. The symbol names are constructed from the class & ivar name so they are a convenient way to name the data you are looking for. The lldb does "look for the symbol and if you don't find it ask the runtime if it knows where the equivalent data lives directly in the runtime.". The latter task is "LookupRuntimeSymbol".

We don't do it this way in swift, instead we ask RemoteAST for this sort of data.