Skip to content

Implement ReflectionContext::readInstanceStartFromTypeRef #71328

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

Merged
merged 1 commit into from
Feb 3, 2024
Merged

Implement ReflectionContext::readInstanceStartFromTypeRef #71328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/swift/Remote/MetadataReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ class MetadataReader {

/// Given a remote pointer to class metadata, attempt to discover its class
/// instance size and whether fields should use the resilient layout strategy.
llvm::Optional<unsigned> readInstanceStartAndAlignmentFromClassMetadata(
llvm::Optional<unsigned> readInstanceStartFromClassMetadata(
StoredPointer MetadataAddress) {
auto meta = readMetadata(MetadataAddress);
if (!meta || meta->getKind() != MetadataKind::Class)
Expand Down
39 changes: 36 additions & 3 deletions include/swift/RemoteInspection/ReflectionContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ class ReflectionContext
// Figure out where the stored properties of this class begin
// by looking at the size of the superclass
auto start =
this->readInstanceStartAndAlignmentFromClassMetadata(MetadataAddress);
this->readInstanceStartFromClassMetadata(MetadataAddress);

// Perform layout
if (start)
Expand Down Expand Up @@ -1202,8 +1202,41 @@ class ReflectionContext
}
}

const RecordTypeInfo *getRecordTypeInfo(const TypeRef *TR,
remote::TypeInfoProvider *ExternalTypeInfo) {
/// Given a typeref, attempt to calculate the unaligned start of this
/// instance's fields. For example, for a type without a superclass, the start
/// of the instance fields would after the word for the isa pointer and the
/// word for the refcount field. For a subclass the start would be the after
/// the superclass's fields. For a version of this function that performs the
/// same job but starting out with an instance pointer check
/// MetadataReader::readInstanceStartFromClassMetadata.
llvm::Optional<unsigned>
computeUnalignedFieldStartOffset(const TypeRef *TR) {
size_t isaAndRetainCountSize = sizeof(StoredSize) + sizeof(long long);

const TypeRef *superclass = getBuilder().lookupSuperclass(TR);
if (!superclass)
// If there is no superclass the stat of the instance's field is right
// after the isa and retain fields.
return isaAndRetainCountSize;

auto superclassStart =
computeUnalignedFieldStartOffset(superclass);
if (!superclassStart)
return llvm::None;

auto *superTI = getBuilder().getTypeConverter().getClassInstanceTypeInfo(
superclass, *superclassStart, nullptr);
if (!superTI)
return llvm::None;

// The start of the subclass's fields is right after the super class's ones.
size_t start = superTI->getSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

This might need to account for padding. For example:

class Sup {
  var n: Int8
}

class Sub {
  var s: String
}

This might give you 17 (on 64-bit) but Sub's field starts at 24. Or for a more painful case:

class Sup {
  var n: Int
}

class Sub {
  var m: SIMD4<Double>
}

This will give you 24 but m is 16-byte aligned and starts at 32.

Copy link
Contributor Author

@augusto2112 augusto2112 Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I tested out your examples and you're right Mike, but it looks like the where I'm using this expects the size of the superclass (except if there's no superclass, then it expects the start of the fields offset), and accounts for padding correctly, so I'll update the function name and comment to reflect this better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up with readSizeOfFieldsNotBelongingToInstanceFromTypeRef although I'm not in love with the name. It's kind of hard to describe "the size of the superclass if there is one or the start of this instance's fields if there isn't" in a concise function name...

Copy link
Contributor

Choose a reason for hiding this comment

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

The name is tough here. How about readUnalignedFieldStartOffsetFromTypeRef? The "unaligned" tells the caller that it's not necessarily correct, depending on the alignment of the first field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think that's better

return start;
}

const RecordTypeInfo *
getRecordTypeInfo(const TypeRef *TR,
remote::TypeInfoProvider *ExternalTypeInfo) {
auto *TypeInfo = getTypeInfo(TR, ExternalTypeInfo);
return dyn_cast_or_null<const RecordTypeInfo>(TypeInfo);
}
Expand Down