From 8949970bee5f6ae47051b36da99f02a596a3acbd Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 5 Dec 2024 11:30:28 +0100 Subject: [PATCH 1/2] [profile] Fix bounds checks in profile merging These bounds checks work on the result of the pointer addition -- but the pointer addition already asserts that no overflow may occur, so the checks are optimized away after #118472. Avoid this by performing the addition in a way that permits overflow. --- compiler-rt/lib/profile/InstrProfilingMerge.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c index 7cf1679811eb0..77fc69110c73e 100644 --- a/compiler-rt/lib/profile/InstrProfilingMerge.c +++ b/compiler-rt/lib/profile/InstrProfilingMerge.c @@ -127,6 +127,12 @@ 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) { @@ -154,9 +160,10 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData, SrcCountersStart = (char *)SrcDataEnd; SrcCountersEnd = SrcCountersStart + Header->NumCounters * __llvm_profile_counter_entry_size(); - SrcBitmapStart = SrcCountersEnd + __llvm_profile_get_num_padding_bytes( - SrcCountersEnd - SrcCountersStart); - SrcNameStart = SrcBitmapStart + Header->NumBitmapBytes; + SrcBitmapStart = ptr_add_with_overflow( + SrcCountersEnd, + __llvm_profile_get_num_padding_bytes(SrcCountersEnd - SrcCountersStart)); + SrcNameStart = ptr_add_with_overflow(SrcBitmapStart, Header->NumBitmapBytes); SrcValueProfDataStart = SrcNameStart + getDistanceFromCounterToValueProf(Header); if (SrcNameStart < SrcCountersStart || SrcNameStart < SrcBitmapStart) @@ -200,8 +207,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. - char *SrcCounters = - SrcCountersStart + ((uintptr_t)SrcData->CounterPtr - CountersDelta); + char *SrcCounters = ptr_add_with_overflow( + SrcCountersStart, (uintptr_t)SrcData->CounterPtr - CountersDelta); // CountersDelta needs to be decreased as we advance to the next data // record. CountersDelta -= sizeof(*SrcData); From b6d1b817cbd58e27896fcf84889fa6cfb2335486 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 5 Dec 2024 14:40:19 +0100 Subject: [PATCH 2/2] Adjust one more place and fix warning --- compiler-rt/lib/profile/InstrProfilingMerge.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c index 77fc69110c73e..a42fa74d1788e 100644 --- a/compiler-rt/lib/profile/InstrProfilingMerge.c +++ b/compiler-rt/lib/profile/InstrProfilingMerge.c @@ -207,7 +207,7 @@ 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. - char *SrcCounters = ptr_add_with_overflow( + const char *SrcCounters = ptr_add_with_overflow( SrcCountersStart, (uintptr_t)SrcData->CounterPtr - CountersDelta); // CountersDelta needs to be decreased as we advance to the next data // record. @@ -227,8 +227,8 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData, } } - const char *SrcBitmap = - SrcBitmapStart + ((uintptr_t)SrcData->BitmapPtr - BitmapDelta); + const char *SrcBitmap = ptr_add_with_overflow( + SrcBitmapStart, (uintptr_t)SrcData->BitmapPtr - BitmapDelta); // BitmapDelta also needs to be decreased as we advance to the next data // record. BitmapDelta -= sizeof(*SrcData);