Skip to content

[profile] Perform pointer arithmetic in uintptr_t #118944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2024
Merged

[profile] Perform pointer arithmetic in uintptr_t #118944

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 29 additions & 38 deletions compiler-rt/lib/profile/InstrProfilingMerge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,6 @@ getDistanceFromCounterToValueProf(const __llvm_profile_header *const Header) {
PaddingBytesAfterVNamesSize;
}

// Add offset to pointer without assuming that the addition does not overflow.
// This allows performing bounds checks by checking the result of the addition.
static const char *ptr_add_with_overflow(const char *p, size_t offset) {
return (const char *)((uintptr_t)p + offset);
}

COMPILER_RT_VISIBILITY
int __llvm_profile_merge_from_buffer(const char *ProfileData,
uint64_t ProfileSize) {
Expand All @@ -143,49 +137,46 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
return 1;
}

__llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
__llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
char *SrcCountersStart, *DstCounter;
const char *SrcCountersEnd, *SrcCounter;
const char *SrcBitmapStart;
const char *SrcNameStart;
const char *SrcValueProfDataStart, *SrcValueProfData;
uintptr_t CountersDelta = Header->CountersDelta;
uintptr_t BitmapDelta = Header->BitmapDelta;

SrcDataStart =
__llvm_profile_data *SrcDataStart =
(__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
Header->BinaryIdsSize);
SrcDataEnd = SrcDataStart + Header->NumData;
SrcCountersStart = (char *)SrcDataEnd;
SrcCountersEnd = SrcCountersStart +
Header->NumCounters * __llvm_profile_counter_entry_size();
SrcBitmapStart = ptr_add_with_overflow(
SrcCountersEnd,
__llvm_profile_get_num_padding_bytes(SrcCountersEnd - SrcCountersStart));
SrcNameStart = ptr_add_with_overflow(SrcBitmapStart, Header->NumBitmapBytes);
SrcValueProfDataStart =
__llvm_profile_data *SrcDataEnd = SrcDataStart + Header->NumData;
uintptr_t SrcCountersStart = (uintptr_t)SrcDataEnd;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the explicit cast, do we? Same for DstCounter on line 165.

Suggested change
uintptr_t SrcCountersStart = (uintptr_t)SrcDataEnd;
uintptr_t SrcCountersStart = SrcDataEnd;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SrcDataEnd is a __llvm_profile_data * so it does need a cast. I only switched the parts using (const) char * to uintptr_t.

uintptr_t SrcCountersEnd =
SrcCountersStart +
Header->NumCounters * __llvm_profile_counter_entry_size();
uintptr_t SrcBitmapStart =
SrcCountersEnd +
__llvm_profile_get_num_padding_bytes(SrcCountersEnd - SrcCountersStart);
uintptr_t SrcNameStart = SrcBitmapStart + Header->NumBitmapBytes;
uintptr_t SrcValueProfDataStart =
SrcNameStart + getDistanceFromCounterToValueProf(Header);
if (SrcNameStart < SrcCountersStart || SrcNameStart < SrcBitmapStart)
return 1;

// Merge counters by iterating the entire counter section when data section is
// empty due to correlation.
if (Header->NumData == 0) {
for (SrcCounter = SrcCountersStart,
DstCounter = __llvm_profile_begin_counters();
for (uintptr_t SrcCounter = SrcCountersStart,
DstCounter = (uintptr_t)__llvm_profile_begin_counters();
SrcCounter < SrcCountersEnd;) {
if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
*DstCounter &= *SrcCounter;
*(char *)DstCounter &= *(const char *)SrcCounter;
} else {
*(uint64_t *)DstCounter += *(uint64_t *)SrcCounter;
*(uint64_t *)DstCounter += *(const uint64_t *)SrcCounter;
}
SrcCounter += __llvm_profile_counter_entry_size();
DstCounter += __llvm_profile_counter_entry_size();
}
return 0;
}

__llvm_profile_data *SrcData, *DstData;
uintptr_t SrcValueProfData;
for (SrcData = SrcDataStart,
DstData = (__llvm_profile_data *)__llvm_profile_begin_data(),
SrcValueProfData = SrcValueProfDataStart;
Expand All @@ -194,10 +185,10 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
// address of the data to the start address of the counter. On WIN64,
// CounterPtr is a truncated 32-bit value due to COFF limitation. Sign
// extend CounterPtr to get the original value.
char *DstCounters =
(char *)((uintptr_t)DstData + signextIfWin64(DstData->CounterPtr));
char *DstBitmap =
(char *)((uintptr_t)DstData + signextIfWin64(DstData->BitmapPtr));
uintptr_t DstCounters =
(uintptr_t)DstData + signextIfWin64(DstData->CounterPtr);
uintptr_t DstBitmap =
(uintptr_t)DstData + signextIfWin64(DstData->BitmapPtr);
unsigned NVK = 0;

// SrcData is a serialized representation of the memory image. We need to
Expand All @@ -207,8 +198,8 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
// CountersDelta computes the offset into the in-buffer counter section.
//
// On WIN64, CountersDelta is truncated as well, so no need for signext.
const char *SrcCounters = ptr_add_with_overflow(
SrcCountersStart, (uintptr_t)SrcData->CounterPtr - CountersDelta);
uintptr_t SrcCounters =
SrcCountersStart + ((uintptr_t)SrcData->CounterPtr - CountersDelta);
// CountersDelta needs to be decreased as we advance to the next data
// record.
CountersDelta -= sizeof(*SrcData);
Expand All @@ -221,14 +212,14 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
for (unsigned I = 0; I < NC; I++) {
if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
// A value of zero signifies the function is covered.
DstCounters[I] &= SrcCounters[I];
((char *)DstCounters)[I] &= ((const char *)SrcCounters)[I];
} else {
((uint64_t *)DstCounters)[I] += ((uint64_t *)SrcCounters)[I];
((uint64_t *)DstCounters)[I] += ((const uint64_t *)SrcCounters)[I];
}
}

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

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

if (SrcValueProfData >= ProfileData + ProfileSize)
if (SrcValueProfData >= (uintptr_t)ProfileData + ProfileSize)
return 1;
VPMergeHook((ValueProfData *)SrcValueProfData, DstData);
SrcValueProfData =
Expand Down
Loading