Skip to content

Add extraction of protocol conformance descriptor info, as read from an object file image, to TypeRefBuilder. #41027

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Feb 2, 2022
Merged

Add extraction of protocol conformance descriptor info, as read from an object file image, to TypeRefBuilder. #41027

merged 8 commits into from
Feb 2, 2022

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Jan 26, 2022

This PR adds the ability to read protocol conformance descriptors from object files, in order to get at the type/protocol information.

  • Object file handling/reading is refactored into the static_mirror namespace:
    Code moved from swift-reflection-dump.cpp into StaticMirror/ObjectFileContext.cpp and StaticMirror/ObjectFileContext.h, without changes.
  • ObjectFile.h has been taught about the __swift5_proto sections in relevant object file formats.
  • Metadata.h now also provides type aliases for External versions of Conformance and Protocol descriptors, as opposed to InProcess ones.
  • TypeRefBuilder.h now provides API to query and dump conformance information - dumpConformanceSection, which is templated on pointer size, since we may be reading this info from binaries built for 32-bit or 64-bit platforms.

@artemcm
Add Conformance section to ReflectionInfo read from object files
d6d4216
@artemcm
Factor out Object File handling from swift-reflection-dump to separ…
f3d784f 
…ate CMake target: StaticMirror

This target will then be used to have a broader set of tools for reading metadata from object files.
@artemcm
Add extraction protocol conformance descriptor extraction, as read fr…
e934fe2 
…om an object file image, to `TypeRefBuilder`.
mikeash
mikeash requested changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

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

Some small comments, but this is looking very nice.

@nkcsgexi nkcsgexi requested a review from DougGregor January 27, 2022 17:25
@artemcm
Add support for reading protocol descriptors from an indirect protoco…
59d1960 
…l descriptor target in an conformance descriptor.

Previously, the code assumed that such an indirect target will always point to an external symbol pointer, but it can also be an absolute pointer to an in-image protocol descriptor.
@artemcm
Copy link
Contributor Author

artemcm commented Jan 27, 2022

@swift-ci please test

@artemcm
Fix section name lookup in findMachOSectionByName to do comparison …
f432f1b 
…up-to the 16-byte section name limit, instead of the queried name length.
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 59d1960

@artemcm
Copy link
Contributor Author

artemcm commented Jan 27, 2022

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - f432f1b

@slavapestov
Copy link
Contributor

TypeRefBuilder was originally intended to work even without a remote process. Perhaps ReflectionContext would be a better place for something that reads sections?

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - f432f1b

@artemcm
Copy link
Contributor Author

artemcm commented Jan 28, 2022

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - db5d4f834a2ff74223f1e3ce2dc05d1ec1bbc2d2

@artemcm artemcm force-pushed the AddConformanceMetadataReader branch from db5d4f8 to dcef67f Compare January 28, 2022 07:09
@artemcm
Copy link
Contributor Author

artemcm commented Jan 28, 2022

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Jan 28, 2022

TypeRefBuilder was originally intended to work even without a remote process. Perhaps ReflectionContext would be a better place for something that reads sections?

I agree that ReflectionContext would probably be a better place for some of this functionality; but, moving over all of the section-reading code into there would involve refactoring a lot of other existing code in TypeRefBuilder that reads sections already. If it is alright with you, I'd prefer to tackle that refactor separately.

@artemcm
Extract fully-qualified names for protocol conformances by traversing…
cd6d8f9 
… their parent contexts and extracting their names
@artemcm artemcm force-pushed the AddConformanceMetadataReader branch from dcef67f to cd6d8f9 Compare January 28, 2022 18:52
@artemcm
Copy link
Contributor Author

artemcm commented Jan 28, 2022

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - cd6d8f9

@artemcm
Copy link
Contributor Author

artemcm commented Jan 28, 2022

@swift-ci please test macOS platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - cd6d8f9

@artemcm
[Static Mirror] Add mangled names to image-extracted protocol conform…
1d96037 
…ance infos.

Collect the mapping of unmangled-to-mangled names from the read out `Fields` section reflection infos and use that to identify the mangled name of a conforming type of a given protocol conformance.
@artemcm
Copy link
Contributor Author

artemcm commented Jan 30, 2022

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Jan 31, 2022

@swift-ci please test Windows platform

mikeash
mikeash reviewed Feb 1, 2022
View reviewed changes
include/swift/ABI/Metadata.h Outdated
template<unsigned PointerSize>
using ExternalTypeContextDescriptor
#if SWIFT_OBJC_INTEROP
= TargetTypeContextDescriptor<External<WithObjCInterop<RuntimeTarget<PointerSize>>>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This still isn't quite right, since we want the client to be able to specify whether the target has ObjC interop separately from how the client builds. I believe the right approach here would be to modify ProtocolConformanceDescriptorReader to be templated on ObjC interop as well as pointer size, and do the WithObjCInterop/NoObjCInterop logic there. @adrian-prantl Does that sound right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, makes perfect sense, thank you.
Updated with the type aliases templated on ObjCInteropKind to allow clients to decide on this separately from how this code is built.

@artemcm
[Static Mirror] Allow ObjCInteropKind to be specified by the client o…
5d17074 
…f `conformanceReader`, separately from how the client builds.
@artemcm artemcm force-pushed the AddConformanceMetadataReader branch from d9b3b37 to 5d17074 Compare February 1, 2022 18:57
@artemcm
Copy link
Contributor Author

artemcm commented Feb 1, 2022

@swift-ci please test

@artemcm artemcm merged commit b4926df into swiftlang:main Feb 2, 2022
@nkcsgexi
Copy link
Contributor

nkcsgexi commented Feb 2, 2022

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrian-prantl adrian-prantl adrian-prantl left review comments

@mikeash mikeash mikeash approved these changes

@nkcsgexi nkcsgexi Awaiting requested review from nkcsgexi

@al45tair al45tair Awaiting requested review from al45tair

@augusto2112 augusto2112 Awaiting requested review from augusto2112

@DougGregor DougGregor Awaiting requested review from DougGregor DougGregor is a code owner

@rastogishubham rastogishubham Awaiting requested review from rastogishubham

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@artemcm @swift-ci @slavapestov @nkcsgexi @mikeash @adrian-prantl