Skip to content

Commit caa2d9a

Browse files
committed
[5.9][Reflection] Improve ownsAddress and metadataIsActor logic.
Separately track text and data segments for ownsAddress. We were previously tracking one range per image, encompassing the range from the start of the image through the end of the data segment. This ends up including a lot of unwanted address space if the two aren't adjacent, as is the case for libraries in the shared cache on Darwin. This makes metadataIsActor a lot more reliable, as it was previously identifying a lot of garbage as actor metadata due to the supposed descriptor pointer falling in this range. rdar://113417637 (cherry picked from commit 2faa2d6)
1 parent 2e3c2b7 commit caa2d9a

File tree

1 file changed

+34
-21
lines changed

1 file changed

+34
-21
lines changed

include/swift/RemoteInspection/ReflectionContext.h

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ class ReflectionContext
126126
/// All buffers we need to keep around long term. This will automatically free them
127127
/// when this object is destroyed.
128128
std::vector<MemoryReader::ReadBytesResult> savedBuffers;
129-
std::vector<std::tuple<RemoteAddress, RemoteAddress>> imageRanges;
129+
std::vector<std::tuple<RemoteAddress, RemoteAddress>> textRanges;
130+
std::vector<std::tuple<RemoteAddress, RemoteAddress>> dataRanges;
130131

131132
bool setupTargetPointers = false;
132133
typename super::StoredPointer target_non_future_adapter = 0;
@@ -251,7 +252,7 @@ class ReflectionContext
251252
uint64_t Offset = 0;
252253

253254
// Find the __TEXT segment.
254-
typename T::SegmentCmd *Command = nullptr;
255+
typename T::SegmentCmd *TextCommand = nullptr;
255256
for (unsigned I = 0; I < NumCommands; ++I) {
256257
auto CmdBuf = this->getReader().readBytes(
257258
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
@@ -260,15 +261,15 @@ class ReflectionContext
260261
return false;
261262
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
262263
if (strncmp(CmdHdr->segname, "__TEXT", sizeof(CmdHdr->segname)) == 0) {
263-
Command = CmdHdr;
264+
TextCommand = CmdHdr;
264265
savedBuffers.push_back(std::move(CmdBuf));
265266
break;
266267
}
267268
Offset += CmdHdr->cmdsize;
268269
}
269270

270271
// No __TEXT segment, bail out.
271-
if (!Command)
272+
if (!TextCommand)
272273
return false;
273274

274275
// Find the load command offset.
@@ -291,7 +292,7 @@ class ReflectionContext
291292
if (!Sections)
292293
return false;
293294

294-
auto Slide = ImageStart.getAddressData() - Command->vmaddr;
295+
auto Slide = ImageStart.getAddressData() - TextCommand->vmaddr;
295296
auto SectionsBuf = reinterpret_cast<const char *>(Sections.get());
296297

297298
auto findMachOSectionByName = [&](llvm::StringRef Name)
@@ -355,22 +356,27 @@ class ReflectionContext
355356

356357
auto InfoID = this->addReflectionInfo(info);
357358

358-
// Find the __DATA segment.
359+
auto TextSegmentStart = Slide + TextCommand->vmaddr;
360+
auto TextSegmentEnd = TextSegmentStart + TextCommand->vmsize;
361+
textRanges.push_back(std::make_tuple(RemoteAddress(TextSegmentStart),
362+
RemoteAddress(TextSegmentEnd)));
363+
364+
// Find the __DATA segments.
359365
for (unsigned I = 0; I < NumCommands; ++I) {
360366
auto CmdBuf = this->getReader().readBytes(
361367
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
362368
SegmentCmdHdrSize);
363369
if (!CmdBuf)
364370
return false;
365371
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
366-
if (strncmp(CmdHdr->segname, "__DATA", sizeof(CmdHdr->segname)) == 0) {
367-
auto DataSegmentEnd =
368-
ImageStart.getAddressData() + CmdHdr->vmaddr + CmdHdr->vmsize;
369-
assert(DataSegmentEnd > ImageStart.getAddressData() &&
372+
// Look for any segment name starting with __DATA.
373+
if (strncmp(CmdHdr->segname, "__DATA", 6) == 0) {
374+
auto DataSegmentStart = Slide + CmdHdr->vmaddr;
375+
auto DataSegmentEnd = DataSegmentStart + CmdHdr->vmsize;
376+
assert(DataSegmentStart > ImageStart.getAddressData() &&
370377
"invalid range for __DATA");
371-
imageRanges.push_back(
372-
std::make_tuple(ImageStart, RemoteAddress(DataSegmentEnd)));
373-
break;
378+
dataRanges.push_back(std::make_tuple(RemoteAddress(DataSegmentStart),
379+
RemoteAddress(DataSegmentEnd)));
374380
}
375381
Offset += CmdHdr->cmdsize;
376382
}
@@ -829,9 +835,11 @@ class ReflectionContext
829835
return ownsAddress(RemoteAddress(*MetadataAddress));
830836
}
831837

832-
/// Returns true if the address falls within a registered image.
833-
bool ownsAddressRaw(RemoteAddress Address) {
834-
for (auto Range : imageRanges) {
838+
/// Returns true if the address falls within the given address ranges.
839+
bool ownsAddress(
840+
RemoteAddress Address,
841+
const std::vector<std::tuple<RemoteAddress, RemoteAddress>> &ranges) {
842+
for (auto Range : ranges) {
835843
auto Start = std::get<0>(Range);
836844
auto End = std::get<1>(Range);
837845
if (Start.getAddressData() <= Address.getAddressData()
@@ -843,11 +851,14 @@ class ReflectionContext
843851
}
844852

845853
/// Returns true if the address is known to the reflection context.
846-
/// Currently, that means that either the address falls within a registered
847-
/// image, or the address points to a Metadata whose type context descriptor
848-
/// is within a registered image.
854+
/// Currently, that means that either the address falls within the text or
855+
/// data segments of a registered image, or the address points to a Metadata
856+
/// whose type context descriptor is within the text segment of a registered
857+
/// image.
849858
bool ownsAddress(RemoteAddress Address) {
850-
if (ownsAddressRaw(Address))
859+
if (ownsAddress(Address, textRanges))
860+
return true;
861+
if (ownsAddress(Address, dataRanges))
851862
return true;
852863

853864
// This is usually called on a Metadata address which might have been
@@ -856,7 +867,7 @@ class ReflectionContext
856867
if (auto Metadata = readMetadata(Address.getAddressData()))
857868
if (auto DescriptorAddress =
858869
super::readAddressOfNominalTypeDescriptor(Metadata, true))
859-
if (ownsAddressRaw(RemoteAddress(DescriptorAddress)))
870+
if (ownsAddress(RemoteAddress(DescriptorAddress), textRanges))
860871
return true;
861872

862873
return false;
@@ -1206,6 +1217,8 @@ class ReflectionContext
12061217
super::readAddressOfNominalTypeDescriptor(Metadata);
12071218
if (!DescriptorAddress)
12081219
return false;
1220+
if (!ownsAddress(RemoteAddress(DescriptorAddress), textRanges))
1221+
return false;
12091222

12101223
auto DescriptorBytes =
12111224
getReader().readBytes(RemoteAddress(DescriptorAddress),

0 commit comments

Comments
 (0)