Skip to content

Commit f0609a6

Browse files
committed
Make a temporary existential when reflecting weak properties
When getting a mirror child that is a class existential, there may be witness tables for the protocol composition to copy. Don't just take the address of a class instance pointer from the stack - make a temporary existential-like before calling into the Mirror constructor. This now correctly covers reflecting weak optional class types, and weak optional class existential types, along with fixing a stack buffer overflow reported by the Address Sanitizer (thanks, ASan!). Tests were also updated to check for the validity of the child's data. rdar://problem/27348445
1 parent 47d618c commit f0609a6

File tree

3 files changed

+217
-78
lines changed

3 files changed

+217
-78
lines changed

include/swift/Runtime/Metadata.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ using TargetFarRelativeIndirectablePointer
134134
= typename Runtime::template FarRelativeIndirectablePointer<Pointee,Nullable>;
135135

136136
struct HeapObject;
137+
struct WeakReference;
137138

138139
template <typename Runtime> struct TargetMetadata;
139140
using Metadata = TargetMetadata<InProcess>;
@@ -2327,8 +2328,9 @@ using OpaqueExistentialContainer
23272328
= TargetOpaqueExistentialContainer<InProcess>;
23282329

23292330
/// The basic layout of a class-bounded existential type.
2330-
struct ClassExistentialContainer {
2331-
void *Value;
2331+
template <typename ContainedValue>
2332+
struct ClassExistentialContainerImpl {
2333+
ContainedValue Value;
23322334

23332335
const WitnessTable **getWitnessTables() {
23342336
return reinterpret_cast<const WitnessTable**>(this + 1);
@@ -2337,11 +2339,15 @@ struct ClassExistentialContainer {
23372339
return reinterpret_cast<const WitnessTable* const *>(this + 1);
23382340
}
23392341

2340-
void copyTypeInto(ClassExistentialContainer *dest, unsigned numTables) const {
2342+
void copyTypeInto(ClassExistentialContainerImpl *dest,
2343+
unsigned numTables) const {
23412344
for (unsigned i = 0; i != numTables; ++i)
23422345
dest->getWitnessTables()[i] = getWitnessTables()[i];
23432346
}
23442347
};
2348+
using ClassExistentialContainer = ClassExistentialContainerImpl<void *>;
2349+
using WeakClassExistentialContainer =
2350+
ClassExistentialContainerImpl<WeakReference>;
23452351

23462352
/// The possible physical representations of existential types.
23472353
enum class ExistentialTypeRepresentation {

stdlib/public/runtime/Reflection.mm

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -403,27 +403,55 @@ static bool loadSpecialReferenceStorage(HeapObject *owner,
403403
OpaqueValue *fieldData,
404404
const FieldType fieldType,
405405
Mirror *outMirror) {
406-
// TODO: Switch over the other kinds of reference storage here:
407-
// - unowned(safe)
408-
// - unowned(unsafe)
409-
// - class-bound existentials
410-
// - Optional existential types
411-
// This will require a change in the two low flag bits in the field type
412-
// returned from the field type accessor generated by IRGen.
413-
414406
// isWeak() implies a reference type via Sema.
415407
if (!fieldType.isWeak())
416408
return false;
417409

410+
auto type = fieldType.getType();
411+
assert(type->getKind() == MetadataKind::Optional);
412+
418413
auto weakField = reinterpret_cast<WeakReference *>(fieldData);
419414
auto strongValue = swift_unknownWeakLoadStrong(weakField);
420-
fieldData = reinterpret_cast<OpaqueValue *>(&strongValue);
415+
416+
// Now that we have a strong reference, we need to create a temporary buffer
417+
// from which to copy the whole value, which might be a native class-bound
418+
// existential, which means we also need to copy n witness tables, for
419+
// however many protocols are in the protocol composition. For example, if we
420+
// are copying a:
421+
// weak var myWeakProperty : (Protocol1 & Protocol2)?
422+
// then we need to copy three values:
423+
// - the instance
424+
// - the witness table for Protocol1
425+
// - the witness table for Protocol2
426+
427+
auto weakContainer =
428+
reinterpret_cast<WeakClassExistentialContainer *>(fieldData);
429+
430+
// Create a temporary existential where we can put the strong reference.
431+
// The allocateBuffer value witness requires a ValueBuffer to own the
432+
// allocated storage.
433+
ValueBuffer temporaryBuffer;
434+
435+
auto temporaryValue =
436+
reinterpret_cast<ClassExistentialContainer *>(
437+
type->vw_allocateBuffer(&temporaryBuffer));
438+
439+
// Now copy the entire value out of the parent, which will include the
440+
// witness tables.
441+
temporaryValue->Value = strongValue;
442+
auto valueWitnessesSize = type->getValueWitnesses()->getSize() -
443+
sizeof(WeakClassExistentialContainer);
444+
memcpy(temporaryValue->getWitnessTables(), weakContainer->getWitnessTables(),
445+
valueWitnessesSize);
421446

422447
// This MagicMirror constructor creates a box to hold the loaded refernce
423448
// value, which becomes the new owner for the value.
424-
new (outMirror) MagicMirror(fieldData, fieldType.getType(), /*take*/ true);
449+
new (outMirror) MagicMirror(reinterpret_cast<OpaqueValue *>(temporaryValue),
450+
type, /*take*/ true);
451+
452+
type->vw_deallocateBuffer(&temporaryBuffer);
425453

426-
// However, swift_StructMirror_subscript and swift_ClassMirror_subscript
454+
// swift_StructMirror_subscript and swift_ClassMirror_subscript
427455
// requires that the owner be consumed. Since we have the new heap box as the
428456
// owner now, we need to release the old owner to maintain the contract.
429457
if (owner->metadata->isAnyClass())

0 commit comments

Comments
 (0)