Teach RemoteMirror how to project enum values #30161

tbkka · 2020-03-02T20:12:59Z

This adds two new functions to the SwiftRemoteMirror
facility that support inspecting enum values.

Currently, these support non-payload enums and
single-payload enums, including nested enums and
payloads with struct, tuple, and reference payloads.
In particular, it handles nested Optional types.

TODO: Multi-payload enums use different strategies for
encoding the cases that aren't yet supported by this
code.

Resolves rdar://59961527

/// Projects the value of an enum.
///
/// Takes the address and typeref for an enum and determines the
/// index of the currently-selected case within the enum.
///
/// Returns true iff the enum case could be successfully determined.
/// In particular, note that this code may fail for valid in-memory data
/// if the compiler is using a strategy we do not yet understand.
SWIFT_REMOTE_MIRROR_LINKAGE
int swift_reflection_projectEnumValue(SwiftReflectionContextRef ContextRef,
                                      swift_addr_t EnumAddress,
                                      swift_typeref_t EnumTypeRef,
                                      int *CaseIndex);

/// Finds information about a particular enum case.
///
/// Given an enum typeref and index of a case, returns:
/// * Typeref of the associated payload or zero if there is no payload
/// * Name of the case if known.
///
/// The Name points to a freshly-allocated C string on the heap.  You
/// are responsible for freeing the string (via `free()`) when you are finished.
SWIFT_REMOTE_MIRROR_LINKAGE
int swift_reflection_getEnumCaseTypeRef(SwiftReflectionContextRef ContextRef,
                                        swift_typeref_t EnumTypeRef,
                                        int CaseIndex,
                                        char **CaseName,
                                        swift_typeref_t *PayloadTypeRef);

rdar://problem/31666592

…into tbkka-remoteMirror-projectEnum

This adds two new functions to the SwiftRemoteMirror facility that support inspecting enum values. Currently, these support non-payload enums and single-payload enums, including nested enums and payloads with struct, tuple, and reference payloads. In particular, it handles nested `Optional` types. TODO: Multi-payload enums use different strategies for encoding the cases that aren't yet supported by this code. Resolves rdar://59961527 ``` /// Projects the value of an enum. /// /// Takes the address and typeref for an enum and determines the /// index of the currently-selected case within the enum. /// /// Returns true iff the enum case could be successfully determined. /// In particular, note that this code may fail for valid in-memory data /// if the compiler is using a strategy we do not yet understand. SWIFT_REMOTE_MIRROR_LINKAGE int swift_reflection_projectEnumValue(SwiftReflectionContextRef ContextRef, swift_addr_t EnumAddress, swift_typeref_t EnumTypeRef, uint64_t *CaseIndex); /// Finds information about a particular enum case. /// /// Given an enum typeref and index of a case, returns: /// * Typeref of the associated payload or zero if there is no payload /// * Name of the case if known. /// /// The Name points to a freshly-allocated C string on the heap. You /// are responsible for freeing the string (via `free()`) when you are finished. SWIFT_REMOTE_MIRROR_LINKAGE int swift_reflection_getEnumCaseTypeRef(SwiftReflectionContextRef ContextRef, swift_typeref_t EnumTypeRef, unsigned CaseIndex, char **CaseName, swift_typeref_t *PayloadTypeRef); ```

tbkka · 2020-03-02T20:14:12Z

This is not quite ready to merge: It currently hard-codes some pointer properties that need to be queried from the target.

Data Layout Queries (DLQ) are questions handled by queryDataLayout() about the target environment. This adds two new parameters: DLQ_GetLeastValidPointerValue DLQ_GetObjCReservedLowBits These are the key parameters needed for the pointer XI calculations. TODO: The implementation of the new DLQ parameters is still incomplete.

dcci · 2020-03-03T00:15:00Z

include/swift/Remote/InProcessMemoryReader.h

+    case DLQ_GetObjCReservedLowBits: {
+      auto result = static_cast<uint8_t *>(outBuffer);
+#if __APPLE__ && __x86_64__
+      // ObjC on 64-bit macOS reserves the bottom bit of all pointers.


What about iOS?

As far as I understand, iOS Obj-C reserves high bits, not low bits. So this literally only applies to 64-bit macOS, no other platforms.

Thanks for clarifying.

This is insufficient for cases where you are inspecting an image for another platform, but it's sufficient for the test suites (which are always run on the same environment as the target) and at least serves to document the necessary logic for clients that need to support cross-platform inspection.

tbkka · 2020-03-04T17:56:41Z

include/swift/Reflection/TypeLowering.h

+      return true;
+    }
+    // XXX Has extra inhabitants, so it must be a pointer?
+    return reader.readHeapObjectExtraInhabitantIndex(address, extraInhabitantIndex);


I'm not sure whether a BuiltinTypeInfo with extra inhabitants can ever be a function pointer or whether it's always just a regular pointer to heap data. (There are different rules for managing XIs for function pointers.)

tbkka · 2020-03-04T17:57:25Z

include/swift/Reflection/TypeLowering.h

+      *extraInhabitantIndex = -1;
+      return true;
+    }
+    return reader.readHeapObjectExtraInhabitantIndex(address, extraInhabitantIndex);


I suspect ReferenceTypeInfo is used for both function pointers and heap pointers, but I'm not quite sure how to distinguish those cases.

tbkka · 2020-03-04T19:18:42Z

I think I've worked out the pointer properties as well as I can for now. I'm still checking some details around XI handling for different kinds of pointers, but otherwise I think this is mergeable (pending CI, of course).

tbkka · 2020-03-04T19:18:55Z

@swift-ci Please test macOS

tbkka · 2020-03-04T19:19:03Z

@swift-ci Please test Linux

swift-ci · 2020-03-04T19:21:36Z

Build failed
Swift Test Linux Platform
Git Sha - 0dd6556

swift-ci · 2020-03-04T21:10:09Z

Build failed
Swift Test OS X Platform
Git Sha - 0dd6556

The earlier version had a bad bug (a reused variable) that ended up always using the last field instead of the one with the most extra inhabitants. Using STL algorithms is safer.

For some reason, `TARGET_OS_IOS` is defined to be 1 on iOS and defined to 0 on all other Apple platforms. So you can't just check whether the symbol is defined or not. It probably would suffice to just use the value, but I had headaches a while back with some compiler that insisted on complaining about undefined preprocessor macros, so I've used the horribly conservative `defined(FOO) && FOO` construction here.

…-payload-enums are rejected (since they're not actually supported)

tbkka · 2020-03-06T01:52:07Z

I think this is ready to merge. The tests could be a little more comprehensive, but the API seems to be working correctly for the current tests on both macOS x86_64 and the 32-bit iOS simulator.

tbkka · 2020-03-06T01:52:25Z

@swift-ci Please smoke test Linux

tbkka · 2020-03-06T01:52:32Z

@swift-ci Please test macOS

tbkka · 2020-03-06T02:00:43Z

@swift-ci Please smoke test Linux

tbkka · 2020-03-06T02:00:49Z

@swift-ci Please test macOS

swift-ci · 2020-03-06T02:14:42Z

Build failed
Swift Test OS X Platform
Git Sha - 118efad

Add some missing 32-bit validation. Also, when printing a payload-carrying enum value, just include the type info ("struct foo") instead of the detailed type ref.

tbkka · 2020-03-06T17:41:03Z

@swift-ci Please test macOS

tbkka · 2020-03-06T17:41:22Z

@swift-ci Please smoke test Linux

swift-ci · 2020-03-06T17:45:19Z

Build failed
Swift Test OS X Platform
Git Sha - bb87e63

tbkka · 2020-03-06T19:33:32Z

@swift-ci Please smoke test Linux

rintaro · 2020-03-07T07:36:31Z

This caused test failure in "Swift in the OS" bot
https://ci.swift.org/job/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/1589/console
@tbkka Could you take a look? rdar://problem/60183719

tbkka · 2020-03-07T21:18:01Z

PR #30281 should fix this.

slavapestov · 2020-03-11T21:58:21Z

stdlib/public/Reflection/TypeLowering.cpp

+    if (Fields.size() == 0) {
+      return false;
+    }
+    // Tuples and Structs inherit XIs from their most capacious member


Can we re-use this computation from when we build the TypeInfo?

slavapestov · 2020-03-11T21:58:34Z

stdlib/public/Reflection/TypeLowering.cpp

+    unsigned long PayloadExtraInhabitants = PayloadCase.TI.getNumExtraInhabitants();
+    unsigned discriminator = 0;
+    auto PayloadSize = PayloadCase.TI.getSize();
+    if (NonPayloadCaseCount >= PayloadExtraInhabitants) {


This should also be re-used from when we lower the type

slavapestov · 2020-03-11T21:59:43Z

stdlib/public/SwiftRemoteMirror/SwiftRemoteMirror.cpp

+    // FIXME: Is there a better way to return a string here?
+    // Just returning Case.Name.c_str() doesn't work as the backing data gets
+    // released at the end of this function.
+    *CaseName = strdup(Name.c_str());


Instead of leaking memory here what if all the case names were pre-allocated by the RecordTypeInfo for the enum, and owned by that same TypeInfo so that we can clean them up when the reflection context is deleted? This simplifies the function's contract.

slavapestov · 2020-03-11T22:02:48Z

include/swift/Reflection/ReflectionContext.h

+    return false;
+  }
+
+  bool getEnumCaseTypeRef(const TypeRef *EnumTR,


I think this API is redundant. In the C API, we already have swift_reflection_childOfTypeRef()

mikeash and others added 3 commits January 9, 2020 16:50

[Reflection] Start roughing out a swift_reflection_projectEnum function.

c4ccfef

rdar://problem/31666592

Merge remote-tracking branch 'github-mikeash/reflection-projectenum' …

54de767

…into tbkka-remoteMirror-projectEnum

tbkka requested review from mikeash and slavapestov March 2, 2020 20:12

dcci reviewed Mar 3, 2020

View reviewed changes

tbkka commented Mar 4, 2020

View reviewed changes

tbkka added 2 commits March 4, 2020 10:02

Rename file

0ddcf8f

Use Function ptr XI calculations for Thin Func ptrs

118efad

tbkka added 6 commits March 5, 2020 13:57

Fix finding the most-capacious field

142b5f1

The earlier version had a bad bug (a reused variable) that ended up always using the last field instead of the one with the most extra inhabitants. Using STL algorithms is safer.

Use 64-bit field for 8-byte pointers.

e8db4af

Fix spelling wordSize => WordSize

0abd2ef

Move readExtraInhabitantIndex into the .cpp file

9b72a3e

Flesh out this test with 32-bit and 64-bit options, verify that multi…

89048d9

…-payload-enums are rejected (since they're not actually supported)

Merge branch 'master' into tbkka-remoteMirror-projectEnum

bb87e63

tbkka added 2 commits March 6, 2020 08:19

Merge branch 'master' into tbkka-remoteMirror-projectEnum

2e2ad42

Fill in, simplify test cases

bc2131f

Add some missing 32-bit validation. Also, when printing a payload-carrying enum value, just include the type info ("struct foo") instead of the detailed type ref.

tbkka merged commit 0d361bd into swiftlang:master Mar 6, 2020

tbkka mentioned this pull request Mar 11, 2020

RemoteMirror: Project some multi-payload enums #30357

Merged

slavapestov reviewed Mar 11, 2020

View reviewed changes

Teach RemoteMirror how to project enum values #30161

Teach RemoteMirror how to project enum values #30161

Uh oh!

Conversation

tbkka commented Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbkka commented Mar 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tbkka commented Mar 4, 2020

Uh oh!

tbkka commented Mar 4, 2020

Uh oh!

tbkka commented Mar 4, 2020

Uh oh!

swift-ci commented Mar 4, 2020

Uh oh!

swift-ci commented Mar 4, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

swift-ci commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

swift-ci commented Mar 6, 2020

Uh oh!

tbkka commented Mar 6, 2020

Uh oh!

rintaro commented Mar 7, 2020

Uh oh!

tbkka commented Mar 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tbkka commented Mar 2, 2020 •

edited

Loading