Skip to content

Commit 2faa2d6

Browse files
committed
[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
1 parent 7b32ffe commit 2faa2d6

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
@@ -128,7 +128,8 @@ class ReflectionContext
128128
/// All buffers we need to keep around long term. This will automatically free them
129129
/// when this object is destroyed.
130130
std::vector<MemoryReader::ReadBytesResult> savedBuffers;
131-
std::vector<std::tuple<RemoteAddress, RemoteAddress>> imageRanges;
131+
std::vector<std::tuple<RemoteAddress, RemoteAddress>> textRanges;
132+
std::vector<std::tuple<RemoteAddress, RemoteAddress>> dataRanges;
132133

133134
bool setupTargetPointers = false;
134135
typename super::StoredPointer target_non_future_adapter = 0;
@@ -253,7 +254,7 @@ class ReflectionContext
253254
uint64_t Offset = 0;
254255

255256
// Find the __TEXT segment.
256-
typename T::SegmentCmd *Command = nullptr;
257+
typename T::SegmentCmd *TextCommand = nullptr;
257258
for (unsigned I = 0; I < NumCommands; ++I) {
258259
auto CmdBuf = this->getReader().readBytes(
259260
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
@@ -262,15 +263,15 @@ class ReflectionContext
262263
return false;
263264
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
264265
if (strncmp(CmdHdr->segname, "__TEXT", sizeof(CmdHdr->segname)) == 0) {
265-
Command = CmdHdr;
266+
TextCommand = CmdHdr;
266267
savedBuffers.push_back(std::move(CmdBuf));
267268
break;
268269
}
269270
Offset += CmdHdr->cmdsize;
270271
}
271272

272273
// No __TEXT segment, bail out.
273-
if (!Command)
274+
if (!TextCommand)
274275
return false;
275276

276277
// Find the load command offset.
@@ -293,7 +294,7 @@ class ReflectionContext
293294
if (!Sections)
294295
return false;
295296

296-
auto Slide = ImageStart.getAddressData() - Command->vmaddr;
297+
auto Slide = ImageStart.getAddressData() - TextCommand->vmaddr;
297298
auto SectionsBuf = reinterpret_cast<const char *>(Sections.get());
298299

299300
auto findMachOSectionByName = [&](llvm::StringRef Name)
@@ -357,22 +358,27 @@ class ReflectionContext
357358

358359
auto InfoID = this->addReflectionInfo(info);
359360

360-
// Find the __DATA segment.
361+
auto TextSegmentStart = Slide + TextCommand->vmaddr;
362+
auto TextSegmentEnd = TextSegmentStart + TextCommand->vmsize;
363+
textRanges.push_back(std::make_tuple(RemoteAddress(TextSegmentStart),
364+
RemoteAddress(TextSegmentEnd)));
365+
366+
// Find the __DATA segments.
361367
for (unsigned I = 0; I < NumCommands; ++I) {
362368
auto CmdBuf = this->getReader().readBytes(
363369
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
364370
SegmentCmdHdrSize);
365371
if (!CmdBuf)
366372
return false;
367373
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
368-
if (strncmp(CmdHdr->segname, "__DATA", sizeof(CmdHdr->segname)) == 0) {
369-
auto DataSegmentEnd =
370-
ImageStart.getAddressData() + CmdHdr->vmaddr + CmdHdr->vmsize;
371-
assert(DataSegmentEnd > ImageStart.getAddressData() &&
374+
// Look for any segment name starting with __DATA.
375+
if (strncmp(CmdHdr->segname, "__DATA", 6) == 0) {
376+
auto DataSegmentStart = Slide + CmdHdr->vmaddr;
377+
auto DataSegmentEnd = DataSegmentStart + CmdHdr->vmsize;
378+
assert(DataSegmentStart > ImageStart.getAddressData() &&
372379
"invalid range for __DATA");
373-
imageRanges.push_back(
374-
std::make_tuple(ImageStart, RemoteAddress(DataSegmentEnd)));
375-
break;
380+
dataRanges.push_back(std::make_tuple(RemoteAddress(DataSegmentStart),
381+
RemoteAddress(DataSegmentEnd)));
376382
}
377383
Offset += CmdHdr->cmdsize;
378384
}
@@ -831,9 +837,11 @@ class ReflectionContext
831837
return ownsAddress(RemoteAddress(*MetadataAddress));
832838
}
833839

834-
/// Returns true if the address falls within a registered image.
835-
bool ownsAddressRaw(RemoteAddress Address) {
836-
for (auto Range : imageRanges) {
840+
/// Returns true if the address falls within the given address ranges.
841+
bool ownsAddress(
842+
RemoteAddress Address,
843+
const std::vector<std::tuple<RemoteAddress, RemoteAddress>> &ranges) {
844+
for (auto Range : ranges) {
837845
auto Start = std::get<0>(Range);
838846
auto End = std::get<1>(Range);
839847
if (Start.getAddressData() <= Address.getAddressData()
@@ -845,11 +853,14 @@ class ReflectionContext
845853
}
846854

847855
/// Returns true if the address is known to the reflection context.
848-
/// Currently, that means that either the address falls within a registered
849-
/// image, or the address points to a Metadata whose type context descriptor
850-
/// is within a registered image.
856+
/// Currently, that means that either the address falls within the text or
857+
/// data segments of a registered image, or the address points to a Metadata
858+
/// whose type context descriptor is within the text segment of a registered
859+
/// image.
851860
bool ownsAddress(RemoteAddress Address) {
852-
if (ownsAddressRaw(Address))
861+
if (ownsAddress(Address, textRanges))
862+
return true;
863+
if (ownsAddress(Address, dataRanges))
853864
return true;
854865

855866
// This is usually called on a Metadata address which might have been
@@ -858,7 +869,7 @@ class ReflectionContext
858869
if (auto Metadata = readMetadata(Address.getAddressData()))
859870
if (auto DescriptorAddress =
860871
super::readAddressOfNominalTypeDescriptor(Metadata, true))
861-
if (ownsAddressRaw(RemoteAddress(DescriptorAddress)))
872+
if (ownsAddress(RemoteAddress(DescriptorAddress), textRanges))
862873
return true;
863874

864875
return false;
@@ -1208,6 +1219,8 @@ class ReflectionContext
12081219
super::readAddressOfNominalTypeDescriptor(Metadata);
12091220
if (!DescriptorAddress)
12101221
return false;
1222+
if (!ownsAddress(RemoteAddress(DescriptorAddress), textRanges))
1223+
return false;
12111224

12121225
auto DescriptorBytes =
12131226
getReader().readBytes(RemoteAddress(DescriptorAddress),

0 commit comments

Comments
 (0)