Skip to content

Commit c288fc9

Browse files
[memprof] Add access checks to PortableMemInfoBlock::get*
commit 4c8ec8f Author: Kazu Hirata <[email protected]> Date: Wed Apr 24 16:25:35 2024 -0700 introduced the idea of serializing/deserializing a subset of the fields in PortableMemInfoBlock. While it reduces the size of the indexed MemProf profile file, we now could inadvertently access unavailable fields and go without noticing. To protect ourselves from the risk, this patch adds access checks to PortableMemInfoBlock::get* methods by embedding a bit set representing available fields into PortableMemInfoBlock.
1 parent 3526020 commit c288fc9

File tree

3 files changed

+32
-12
lines changed

3 files changed

+32
-12
lines changed

llvm/include/llvm/ProfileData/MemProf.h

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#define LLVM_PROFILEDATA_MEMPROF_H_
33

44
#include "llvm/ADT/MapVector.h"
5+
#include "llvm/ADT/STLForwardCompat.h"
56
#include "llvm/ADT/STLFunctionalExtras.h"
67
#include "llvm/ADT/SmallVector.h"
78
#include "llvm/IR/GlobalValue.h"
@@ -10,6 +11,7 @@
1011
#include "llvm/Support/EndianStream.h"
1112
#include "llvm/Support/raw_ostream.h"
1213

14+
#include <bitset>
1315
#include <cstdint>
1416
#include <optional>
1517

@@ -55,7 +57,10 @@ MemProfSchema getHotColdSchema();
5557
// deserialize methods.
5658
struct PortableMemInfoBlock {
5759
PortableMemInfoBlock() = default;
58-
explicit PortableMemInfoBlock(const MemInfoBlock &Block) {
60+
explicit PortableMemInfoBlock(const MemInfoBlock &Block,
61+
const MemProfSchema &IncomingSchema) {
62+
for (const Meta Id : IncomingSchema)
63+
Schema.set(llvm::to_underlying(Id));
5964
#define MIBEntryDef(NameTag, Name, Type) Name = Block.Name;
6065
#include "llvm/ProfileData/MIBEntryDef.inc"
6166
#undef MIBEntryDef
@@ -67,10 +72,12 @@ struct PortableMemInfoBlock {
6772

6873
// Read the contents of \p Ptr based on the \p Schema to populate the
6974
// MemInfoBlock member.
70-
void deserialize(const MemProfSchema &Schema, const unsigned char *Ptr) {
75+
void deserialize(const MemProfSchema &IncomingSchema,
76+
const unsigned char *Ptr) {
7177
using namespace support;
7278

73-
for (const Meta Id : Schema) {
79+
Schema.reset();
80+
for (const Meta Id : IncomingSchema) {
7481
switch (Id) {
7582
#define MIBEntryDef(NameTag, Name, Type) \
7683
case Meta::Name: { \
@@ -82,6 +89,8 @@ struct PortableMemInfoBlock {
8289
llvm_unreachable("Unknown meta type id, is the profile collected from "
8390
"a newer version of the runtime?");
8491
}
92+
93+
Schema.set(llvm::to_underlying(Id));
8594
}
8695
}
8796

@@ -116,15 +125,22 @@ struct PortableMemInfoBlock {
116125

117126
// Define getters for each type which can be called by analyses.
118127
#define MIBEntryDef(NameTag, Name, Type) \
119-
Type get##Name() const { return Name; }
128+
Type get##Name() const { \
129+
assert(Schema[llvm::to_underlying(Meta::Name)]); \
130+
return Name; \
131+
}
120132
#include "llvm/ProfileData/MIBEntryDef.inc"
121133
#undef MIBEntryDef
122134

123135
void clear() { *this = PortableMemInfoBlock(); }
124136

125137
bool operator==(const PortableMemInfoBlock &Other) const {
138+
if (Other.Schema != Schema)
139+
return false;
140+
126141
#define MIBEntryDef(NameTag, Name, Type) \
127-
if (Other.get##Name() != get##Name()) \
142+
if (Schema[llvm::to_underlying(Meta::Name)] && \
143+
Other.get##Name() != get##Name()) \
128144
return false;
129145
#include "llvm/ProfileData/MIBEntryDef.inc"
130146
#undef MIBEntryDef
@@ -155,6 +171,9 @@ struct PortableMemInfoBlock {
155171
}
156172

157173
private:
174+
// The set of available fields, indexed by Meta::Name.
175+
std::bitset<llvm::to_underlying(Meta::Size)> Schema;
176+
158177
#define MIBEntryDef(NameTag, Name, Type) Type Name = Type();
159178
#include "llvm/ProfileData/MIBEntryDef.inc"
160179
#undef MIBEntryDef
@@ -296,8 +315,9 @@ struct IndexedAllocationInfo {
296315

297316
IndexedAllocationInfo() = default;
298317
IndexedAllocationInfo(ArrayRef<FrameId> CS, CallStackId CSId,
299-
const MemInfoBlock &MB)
300-
: CallStack(CS.begin(), CS.end()), CSId(CSId), Info(MB) {}
318+
const MemInfoBlock &MB,
319+
const MemProfSchema &Schema = getFullSchema())
320+
: CallStack(CS.begin(), CS.end()), CSId(CSId), Info(MB, Schema) {}
301321

302322
// Returns the size in bytes when this allocation info struct is serialized.
303323
size_t serializedSize(const MemProfSchema &Schema,

llvm/unittests/ProfileData/InstrProfTest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -407,13 +407,13 @@ IndexedMemProfRecord makeRecord(
407407
IndexedMemProfRecord
408408
makeRecordV2(std::initializer_list<::llvm::memprof::CallStackId> AllocFrames,
409409
std::initializer_list<::llvm::memprof::CallStackId> CallSiteFrames,
410-
const MemInfoBlock &Block) {
410+
const MemInfoBlock &Block, const memprof::MemProfSchema &Schema) {
411411
llvm::memprof::IndexedMemProfRecord MR;
412412
for (const auto &CSId : AllocFrames)
413413
// We don't populate IndexedAllocationInfo::CallStack because we use it only
414414
// in Version0 and Version1.
415415
MR.AllocSites.emplace_back(::llvm::SmallVector<memprof::FrameId>(), CSId,
416-
Block);
416+
Block, Schema);
417417
for (const auto &CSId : CallSiteFrames)
418418
MR.CallSiteIds.push_back(CSId);
419419
return MR;
@@ -506,7 +506,7 @@ TEST_F(InstrProfTest, test_memprof_v2_full_schema) {
506506

507507
const IndexedMemProfRecord IndexedMR = makeRecordV2(
508508
/*AllocFrames=*/{0x111, 0x222},
509-
/*CallSiteFrames=*/{0x333}, MIB);
509+
/*CallSiteFrames=*/{0x333}, MIB, memprof::getFullSchema());
510510
const FrameIdMapTy IdToFrameMap = getFrameMapping();
511511
const auto CSIdToCallStackMap = getCallStackMapping();
512512
for (const auto &I : IdToFrameMap) {
@@ -548,7 +548,7 @@ TEST_F(InstrProfTest, test_memprof_v2_partial_schema) {
548548

549549
const IndexedMemProfRecord IndexedMR = makeRecordV2(
550550
/*AllocFrames=*/{0x111, 0x222},
551-
/*CallSiteFrames=*/{0x333}, MIB);
551+
/*CallSiteFrames=*/{0x333}, MIB, memprof::getHotColdSchema());
552552
const FrameIdMapTy IdToFrameMap = getFrameMapping();
553553
const auto CSIdToCallStackMap = getCallStackMapping();
554554
for (const auto &I : IdToFrameMap) {

llvm/unittests/ProfileData/MemProfTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ TEST(MemProf, PortableWrapper) {
241241
/*dealloc_cpu=*/4);
242242

243243
const auto Schema = llvm::memprof::getFullSchema();
244-
PortableMemInfoBlock WriteBlock(Info);
244+
PortableMemInfoBlock WriteBlock(Info, Schema);
245245

246246
std::string Buffer;
247247
llvm::raw_string_ostream OS(Buffer);

0 commit comments

Comments
 (0)