Skip to content

Commit

Permalink
[PA] Improve formatting (6 of N)
Browse files Browse the repository at this point in the history
Change-Id: Idd9234d488d0a10044838192aec5c6724758b161
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4322758
Reviewed-by: Kalvin Lee <kdlee@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115568}
  • Loading branch information
bartekn-chromium authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent 8d34808 commit dd4f5f1
Show file tree
Hide file tree
Showing 15 changed files with 95 additions and 49 deletions.
27 changes: 18 additions & 9 deletions base/allocator/partition_allocator/address_pool_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,18 @@ void AddressPoolManager::GetPoolUsedSuperPages(
pool_handle handle,
std::bitset<kMaxSuperPagesInPool>& used) {
Pool* pool = GetPool(handle);
if (!pool)
if (!pool) {
return;
}

pool->GetUsedSuperPages(used);
}

uintptr_t AddressPoolManager::GetPoolBaseAddress(pool_handle handle) {
Pool* pool = GetPool(handle);
if (!pool)
if (!pool) {
return 0;
}

return pool->GetBaseAddress();
}
Expand All @@ -92,11 +94,13 @@ uintptr_t AddressPoolManager::Reserve(pool_handle handle,
uintptr_t requested_address,
size_t length) {
Pool* pool = GetPool(handle);
if (!requested_address)
if (!requested_address) {
return pool->FindChunk(length);
}
const bool is_available = pool->TryReserveChunk(requested_address, length);
if (is_available)
if (is_available) {
return requested_address;
}
return pool->FindChunk(length);
}

Expand Down Expand Up @@ -163,8 +167,9 @@ uintptr_t AddressPoolManager::Pool::FindChunk(size_t requested_size) {
// |end_bit| points 1 past the last bit that needs to be 0. If it goes past
// |total_bits_|, return |nullptr| to signal no free chunk was found.
size_t end_bit = beg_bit + need_bits;
if (end_bit > total_bits_)
if (end_bit > total_bits_) {
return 0;
}

bool found = true;
for (; curr_bit < end_bit; ++curr_bit) {
Expand All @@ -176,8 +181,9 @@ uintptr_t AddressPoolManager::Pool::FindChunk(size_t requested_size) {
// next outer loop pass from checking the same bits.
beg_bit = curr_bit + 1;
found = false;
if (bit_hint_ == curr_bit)
if (bit_hint_ == curr_bit) {
++bit_hint_;
}
}
}

Expand Down Expand Up @@ -212,12 +218,14 @@ bool AddressPoolManager::Pool::TryReserveChunk(uintptr_t address,
const size_t need_bits = requested_size / kSuperPageSize;
const size_t end_bit = begin_bit + need_bits;
// Check that requested address is not too high.
if (end_bit > total_bits_)
if (end_bit > total_bits_) {
return false;
}
// Check if any bit of the requested region is set already.
for (size_t i = begin_bit; i < end_bit; ++i) {
if (alloc_bitset_.test(i))
if (alloc_bitset_.test(i)) {
return false;
}
}
// Otherwise, set the bits.
for (size_t i = begin_bit; i < end_bit; ++i) {
Expand Down Expand Up @@ -520,8 +528,9 @@ bool AddressPoolManager::GetStats(AddressSpaceStats* stats) {
// Get blocklist size.
for (const auto& blocked :
AddressPoolManagerBitmap::brp_forbidden_super_page_map_) {
if (blocked.load(std::memory_order_relaxed))
if (blocked.load(std::memory_order_relaxed)) {
stats->blocklist_size += 1;
}
}

// Count failures in finding non-blocklisted addresses.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ uintptr_t GetRandomPageBase() {
// randomization isn't buying anything. In that case we just skip it.
// TODO(palmer): Just dump the randomization when HE-ASLR is present.
static BOOL is_wow64 = -1;
if (is_wow64 == -1 && !IsWow64Process(GetCurrentProcess(), &is_wow64))
if (is_wow64 == -1 && !IsWow64Process(GetCurrentProcess(), &is_wow64)) {
is_wow64 = FALSE;
if (!is_wow64)
}
if (!is_wow64) {
return 0;
}
#endif // BUILDFLAG(IS_WIN)
random &= internal::ASLRMask();
random += internal::ASLROffset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ uintptr_t GetMask() {
#elif defined(ARCH_CPU_32_BITS)
#if BUILDFLAG(IS_WIN)
BOOL is_wow64 = FALSE;
if (!IsWow64Process(GetCurrentProcess(), &is_wow64))
if (!IsWow64Process(GetCurrentProcess(), &is_wow64)) {
is_wow64 = FALSE;
}
if (!is_wow64) {
mask = 0;
}
Expand Down Expand Up @@ -67,8 +68,9 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, DisabledASLR) {

TEST(PartitionAllocAddressSpaceRandomizationTest, Alignment) {
uintptr_t mask = GetMask();
if (!mask)
if (!mask) {
return;
}

for (size_t i = 0; i < kSamples; ++i) {
uintptr_t address = GetAddressBits();
Expand All @@ -79,8 +81,9 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, Alignment) {

TEST(PartitionAllocAddressSpaceRandomizationTest, Range) {
uintptr_t mask = GetMask();
if (!mask)
if (!mask) {
return;
}

uintptr_t min = internal::ASLROffset();
uintptr_t max = internal::ASLROffset() + internal::ASLRMask();
Expand All @@ -93,8 +96,9 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, Range) {

TEST(PartitionAllocAddressSpaceRandomizationTest, Predictable) {
uintptr_t mask = GetMask();
if (!mask)
if (!mask) {
return;
}

const uint64_t kInitialSeed = 0xfeed5eedULL;
SetMmapSeedForTesting(kInitialSeed);
Expand Down Expand Up @@ -126,8 +130,9 @@ double ChiSquared(int m, int n) {
// biased.
void RandomBitCorrelation(int random_bit) {
uintptr_t mask = GetMask();
if ((mask & (1ULL << random_bit)) == 0)
if ((mask & (1ULL << random_bit)) == 0) {
return; // bit is always 0.
}

#if BUILDFLAG(PA_DCHECK_IS_ON)
// Do fewer checks when BUILDFLAG(PA_DCHECK_IS_ON). Exercized code only
Expand All @@ -149,8 +154,9 @@ void RandomBitCorrelation(int random_bit) {
// The predicted bit is one of the bits from the PRNG.
for (int ago = 0; ago < kHistory; ago++) {
// We don't want to check whether each bit predicts itself.
if (ago == 0 && predictor_bit == random_bit)
if (ago == 0 && predictor_bit == random_bit) {
continue;
}

// Enter the new random value into the history.
for (int i = ago; i >= 0; i--) {
Expand All @@ -161,8 +167,9 @@ void RandomBitCorrelation(int random_bit) {
int m = 0;
for (int i = 0; i < kRepeats; i++) {
uintptr_t random = GetRandomBits();
for (int j = ago - 1; j >= 0; j--)
for (int j = ago - 1; j >= 0; j--) {
history[j + 1] = history[j];
}
history[0] = random;

int predicted;
Expand All @@ -172,8 +179,9 @@ void RandomBitCorrelation(int random_bit) {
predicted = predictor_bit == -2 ? 0 : 1;
}
int bit = (random >> random_bit) & 1;
if (bit == predicted)
if (bit == predicted) {
m++;
}
}

// Chi squared analysis for k = 2 (2, states: same/not-same) and one
Expand All @@ -185,8 +193,9 @@ void RandomBitCorrelation(int random_bit) {
PA_CHECK(chi_squared <= 35.0);
// If the predictor bit is a fixed 0 or 1 then it makes no sense to
// repeat the test with a different age.
if (predictor_bit < 0)
if (predictor_bit < 0) {
break;
}
}
}
}
Expand Down Expand Up @@ -262,8 +271,9 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, CanMapInAslrRange) {
ASSERT_NE(address, 0u);
FreePages(address, size);

if (address == requested_address)
if (address == requested_address) {
break;
}
}

EXPECT_LT(tries, kMaxTries);
Expand Down
3 changes: 2 additions & 1 deletion base/allocator/partition_allocator/allocation_guard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ thread_local bool g_disallow_allocations;
} // namespace

ScopedDisallowAllocations::ScopedDisallowAllocations() {
if (g_disallow_allocations)
if (g_disallow_allocations) {
PA_IMMEDIATE_CRASH();
}

g_disallow_allocations = true;
}
Expand Down
4 changes: 2 additions & 2 deletions base/allocator/partition_allocator/allocation_guard.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) ScopedAllowAllocations {

#else

struct [[maybe_unused]] ScopedDisallowAllocations{};
struct [[maybe_unused]] ScopedAllowAllocations{};
struct [[maybe_unused]] ScopedDisallowAllocations {};
struct [[maybe_unused]] ScopedAllowAllocations {};

#endif // PA_CONFIG(HAS_ALLOCATION_GUARD)

Expand Down
3 changes: 2 additions & 1 deletion base/allocator/partition_allocator/compressed_pointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ class PA_TRIVIAL_ABI CompressedPointer final {
// frequent operation, we let more work here in favor of faster
// decompression.
// TODO(1376980): Avoid this by overreserving the heap.
if (compressed)
if (compressed) {
compressed |= (1u << (sizeof(uint32_t) * CHAR_BIT - 1));
}

return compressed;
}
Expand Down
9 changes: 6 additions & 3 deletions base/allocator/partition_allocator/extended_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ namespace {
void DisableThreadCacheForRootIfEnabled(ThreadSafePartitionRoot* root) {
// Some platforms don't have a thread cache, or it could already have been
// disabled.
if (!root || !root->flags.with_thread_cache)
if (!root || !root->flags.with_thread_cache) {
return;
}

ThreadCacheRegistry::Instance().PurgeAll();
root->flags.with_thread_cache = false;
Expand All @@ -30,8 +31,9 @@ void DisableThreadCacheForRootIfEnabled(ThreadSafePartitionRoot* root) {

void EnablePartitionAllocThreadCacheForRootIfDisabled(
ThreadSafePartitionRoot* root) {
if (!root)
if (!root) {
return;
}
root->flags.with_thread_cache = true;
}

Expand All @@ -42,8 +44,9 @@ void DisablePartitionAllocThreadCacheForProcess() {
auto* aligned_allocator =
allocator_shim::internal::PartitionAllocMalloc::AlignedAllocator();
DisableThreadCacheForRootIfEnabled(regular_allocator);
if (aligned_allocator != regular_allocator)
if (aligned_allocator != regular_allocator) {
DisableThreadCacheForRootIfEnabled(aligned_allocator);
}
DisableThreadCacheForRootIfEnabled(
allocator_shim::internal::PartitionAllocMalloc::OriginalAllocator());
}
Expand Down
3 changes: 2 additions & 1 deletion base/allocator/partition_allocator/gwp_asan_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ void* GwpAsanSupport::MapRegion(size_t slot_count,
super_page_span_start = bucket->AllocNewSuperPageSpanForGwpAsan(
root.get(), super_page_count, 0);

if (!super_page_span_start)
if (!super_page_span_start) {
return nullptr;
}

#if defined(ARCH_CPU_64_BITS)
// Mapping the GWP-ASan region in to the lower 32-bits of address space
Expand Down
3 changes: 2 additions & 1 deletion base/allocator/partition_allocator/oom_callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ void SetPartitionAllocOomCallback(PartitionAllocOomCallback callback) {

namespace internal {
void RunPartitionAllocOomCallback() {
if (g_oom_callback)
if (g_oom_callback) {
g_oom_callback();
}
}
} // namespace internal

Expand Down
6 changes: 4 additions & 2 deletions base/allocator/partition_allocator/pkey.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,19 @@ void Wrpkru(uint32_t pkru) {

LiftPkeyRestrictionsScope::LiftPkeyRestrictionsScope()
: saved_pkey_value_(kDefaultPkeyValue) {
if (!PkeySettings::settings.enabled)
if (!PkeySettings::settings.enabled) {
return;
}
saved_pkey_value_ = Rdpkru();
if (saved_pkey_value_ != kDefaultPkeyValue) {
Wrpkru(kAllowAllPkeyValue);
}
}

LiftPkeyRestrictionsScope::~LiftPkeyRestrictionsScope() {
if (!PkeySettings::settings.enabled)
if (!PkeySettings::settings.enabled) {
return;
}
if (Rdpkru() != saved_pkey_value_) {
Wrpkru(saved_pkey_value_);
}
Expand Down
27 changes: 18 additions & 9 deletions base/allocator/partition_allocator/pkey_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,30 @@ struct IsolatedGlobals {

int ProtFromSegmentFlags(ElfW(Word) flags) {
int prot = 0;
if (flags & PF_R)
if (flags & PF_R) {
prot |= PROT_READ;
if (flags & PF_W)
}
if (flags & PF_W) {
prot |= PROT_WRITE;
if (flags & PF_X)
}
if (flags & PF_X) {
prot |= PROT_EXEC;
}
return prot;
}

int ProtectROSegments(struct dl_phdr_info* info, size_t info_size, void* data) {
if (!strcmp(info->dlpi_name, "linux-vdso.so.1"))
if (!strcmp(info->dlpi_name, "linux-vdso.so.1")) {
return 0;
}
for (int i = 0; i < info->dlpi_phnum; i++) {
const ElfW(Phdr)* phdr = &info->dlpi_phdr[i];
if (phdr->p_type != PT_LOAD && phdr->p_type != PT_GNU_RELRO)
if (phdr->p_type != PT_LOAD && phdr->p_type != PT_GNU_RELRO) {
continue;
if (phdr->p_flags & PF_W)
}
if (phdr->p_flags & PF_W) {
continue;
}
uintptr_t start = info->dlpi_addr + phdr->p_vaddr;
uintptr_t end = start + phdr->p_memsz;
uintptr_t startPage = RoundDownToSystemPage(start);
Expand Down Expand Up @@ -90,8 +96,9 @@ class PkeyTest : public testing::Test {

void SetUp() override {
int pkey = PkeyAlloc(0);
if (pkey == -1)
if (pkey == -1) {
return;
}
isolatedGlobals.pkey = pkey;

isolatedGlobals.allocator->init({
Expand All @@ -111,8 +118,9 @@ class PkeyTest : public testing::Test {
}

void TearDown() override {
if (isolatedGlobals.pkey == kInvalidPkey)
if (isolatedGlobals.pkey == kInvalidPkey) {
return;
}
PA_PCHECK(PkeyMprotect(&isolatedGlobals, sizeof(isolatedGlobals),
PROT_READ | PROT_WRITE, kDefaultPkey) == 0);
isolatedGlobals.pkey = kDefaultPkey;
Expand Down Expand Up @@ -144,8 +152,9 @@ ISOLATED_FUNCTION uint64_t IsolatedAllocFree(void* arg) {
// order to do this, we need to tag all global read-only memory with our pkey as
// well as switch to a pkey-tagged stack.
TEST_F(PkeyTest, AllocWithoutDefaultPkey) {
if (isolatedGlobals.pkey == kInvalidPkey)
if (isolatedGlobals.pkey == kInvalidPkey) {
return;
}

uint64_t ret;
uint32_t pkru_value = 0;
Expand Down

0 comments on commit dd4f5f1

Please sign in to comment.