Skip to content

Commit fbbea89

Browse files
authored
[profile] Perform pointer arithmetic in uintptr_t (#118944)
Based on the feedback from #118782, this switches most of the pointer arithmetic in __llvm_profile_merge_from_buffer to work on uintptr_t instead of const char *, only casting back to a pointer when performing actual accesses. This ensures that all the arithmetic is performed without any assumptions about pointer overflow.
1 parent 24561f4 commit fbbea89

File tree

1 file changed

+29
-38
lines changed

1 file changed

+29
-38
lines changed

compiler-rt/lib/profile/InstrProfilingMerge.c

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,6 @@ getDistanceFromCounterToValueProf(const __llvm_profile_header *const Header) {
127127
PaddingBytesAfterVNamesSize;
128128
}
129129

130-
// Add offset to pointer without assuming that the addition does not overflow.
131-
// This allows performing bounds checks by checking the result of the addition.
132-
static const char *ptr_add_with_overflow(const char *p, size_t offset) {
133-
return (const char *)((uintptr_t)p + offset);
134-
}
135-
136130
COMPILER_RT_VISIBILITY
137131
int __llvm_profile_merge_from_buffer(const char *ProfileData,
138132
uint64_t ProfileSize) {
@@ -143,49 +137,46 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
143137
return 1;
144138
}
145139

146-
__llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
147140
__llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
148-
char *SrcCountersStart, *DstCounter;
149-
const char *SrcCountersEnd, *SrcCounter;
150-
const char *SrcBitmapStart;
151-
const char *SrcNameStart;
152-
const char *SrcValueProfDataStart, *SrcValueProfData;
153141
uintptr_t CountersDelta = Header->CountersDelta;
154142
uintptr_t BitmapDelta = Header->BitmapDelta;
155143

156-
SrcDataStart =
144+
__llvm_profile_data *SrcDataStart =
157145
(__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
158146
Header->BinaryIdsSize);
159-
SrcDataEnd = SrcDataStart + Header->NumData;
160-
SrcCountersStart = (char *)SrcDataEnd;
161-
SrcCountersEnd = SrcCountersStart +
162-
Header->NumCounters * __llvm_profile_counter_entry_size();
163-
SrcBitmapStart = ptr_add_with_overflow(
164-
SrcCountersEnd,
165-
__llvm_profile_get_num_padding_bytes(SrcCountersEnd - SrcCountersStart));
166-
SrcNameStart = ptr_add_with_overflow(SrcBitmapStart, Header->NumBitmapBytes);
167-
SrcValueProfDataStart =
147+
__llvm_profile_data *SrcDataEnd = SrcDataStart + Header->NumData;
148+
uintptr_t SrcCountersStart = (uintptr_t)SrcDataEnd;
149+
uintptr_t SrcCountersEnd =
150+
SrcCountersStart +
151+
Header->NumCounters * __llvm_profile_counter_entry_size();
152+
uintptr_t SrcBitmapStart =
153+
SrcCountersEnd +
154+
__llvm_profile_get_num_padding_bytes(SrcCountersEnd - SrcCountersStart);
155+
uintptr_t SrcNameStart = SrcBitmapStart + Header->NumBitmapBytes;
156+
uintptr_t SrcValueProfDataStart =
168157
SrcNameStart + getDistanceFromCounterToValueProf(Header);
169158
if (SrcNameStart < SrcCountersStart || SrcNameStart < SrcBitmapStart)
170159
return 1;
171160

172161
// Merge counters by iterating the entire counter section when data section is
173162
// empty due to correlation.
174163
if (Header->NumData == 0) {
175-
for (SrcCounter = SrcCountersStart,
176-
DstCounter = __llvm_profile_begin_counters();
164+
for (uintptr_t SrcCounter = SrcCountersStart,
165+
DstCounter = (uintptr_t)__llvm_profile_begin_counters();
177166
SrcCounter < SrcCountersEnd;) {
178167
if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
179-
*DstCounter &= *SrcCounter;
168+
*(char *)DstCounter &= *(const char *)SrcCounter;
180169
} else {
181-
*(uint64_t *)DstCounter += *(uint64_t *)SrcCounter;
170+
*(uint64_t *)DstCounter += *(const uint64_t *)SrcCounter;
182171
}
183172
SrcCounter += __llvm_profile_counter_entry_size();
184173
DstCounter += __llvm_profile_counter_entry_size();
185174
}
186175
return 0;
187176
}
188177

178+
__llvm_profile_data *SrcData, *DstData;
179+
uintptr_t SrcValueProfData;
189180
for (SrcData = SrcDataStart,
190181
DstData = (__llvm_profile_data *)__llvm_profile_begin_data(),
191182
SrcValueProfData = SrcValueProfDataStart;
@@ -194,10 +185,10 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
194185
// address of the data to the start address of the counter. On WIN64,
195186
// CounterPtr is a truncated 32-bit value due to COFF limitation. Sign
196187
// extend CounterPtr to get the original value.
197-
char *DstCounters =
198-
(char *)((uintptr_t)DstData + signextIfWin64(DstData->CounterPtr));
199-
char *DstBitmap =
200-
(char *)((uintptr_t)DstData + signextIfWin64(DstData->BitmapPtr));
188+
uintptr_t DstCounters =
189+
(uintptr_t)DstData + signextIfWin64(DstData->CounterPtr);
190+
uintptr_t DstBitmap =
191+
(uintptr_t)DstData + signextIfWin64(DstData->BitmapPtr);
201192
unsigned NVK = 0;
202193

203194
// SrcData is a serialized representation of the memory image. We need to
@@ -207,8 +198,8 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
207198
// CountersDelta computes the offset into the in-buffer counter section.
208199
//
209200
// On WIN64, CountersDelta is truncated as well, so no need for signext.
210-
const char *SrcCounters = ptr_add_with_overflow(
211-
SrcCountersStart, (uintptr_t)SrcData->CounterPtr - CountersDelta);
201+
uintptr_t SrcCounters =
202+
SrcCountersStart + ((uintptr_t)SrcData->CounterPtr - CountersDelta);
212203
// CountersDelta needs to be decreased as we advance to the next data
213204
// record.
214205
CountersDelta -= sizeof(*SrcData);
@@ -221,14 +212,14 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
221212
for (unsigned I = 0; I < NC; I++) {
222213
if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
223214
// A value of zero signifies the function is covered.
224-
DstCounters[I] &= SrcCounters[I];
215+
((char *)DstCounters)[I] &= ((const char *)SrcCounters)[I];
225216
} else {
226-
((uint64_t *)DstCounters)[I] += ((uint64_t *)SrcCounters)[I];
217+
((uint64_t *)DstCounters)[I] += ((const uint64_t *)SrcCounters)[I];
227218
}
228219
}
229220

230-
const char *SrcBitmap = ptr_add_with_overflow(
231-
SrcBitmapStart, (uintptr_t)SrcData->BitmapPtr - BitmapDelta);
221+
uintptr_t SrcBitmap =
222+
SrcBitmapStart + ((uintptr_t)SrcData->BitmapPtr - BitmapDelta);
232223
// BitmapDelta also needs to be decreased as we advance to the next data
233224
// record.
234225
BitmapDelta -= sizeof(*SrcData);
@@ -239,7 +230,7 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
239230
return 1;
240231
// Merge Src and Dst Bitmap bytes by simply ORing them together.
241232
for (unsigned I = 0; I < NB; I++)
242-
DstBitmap[I] |= SrcBitmap[I];
233+
((char *)DstBitmap)[I] |= ((const char *)SrcBitmap)[I];
243234
}
244235

245236
/* Now merge value profile data. */
@@ -252,7 +243,7 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
252243
if (!NVK)
253244
continue;
254245

255-
if (SrcValueProfData >= ProfileData + ProfileSize)
246+
if (SrcValueProfData >= (uintptr_t)ProfileData + ProfileSize)
256247
return 1;
257248
VPMergeHook((ValueProfData *)SrcValueProfData, DstData);
258249
SrcValueProfData =

0 commit comments

Comments
 (0)