Skip to content

Commit

Permalink
Revert "Prevent unsafe narrowing: base/ for Linux part 2 of n"
Browse files Browse the repository at this point in the history
This reverts commit 1d7afea.

Reason for revert: Test SysInfoTest.AmountOfAvailablePhysicalMemory fails on android-pie-x86-rel

Original change's description:
> Prevent unsafe narrowing: base/ for Linux part 2 of n
>
> Much of the diff here is from changing the physical/virtual memory funcs
> from signed to unsigned.
>
> Bug: 1292951
> Change-Id: I1a396b97fa25c955198237edf995a628da434aa3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3740962
> Commit-Queue: danakj <danakj@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Owners-Override: danakj <danakj@chromium.org>
> Auto-Submit: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1021898}

Bug: 1292951, 1343625
Change-Id: I2fc9adeb0f5678447e74b9673ec2ec9ac80f95b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3756793
Auto-Submit: Sky Malice <skym@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1023054}
  • Loading branch information
Sky Malice authored and Chromium LUCI CQ committed Jul 12, 2022
1 parent 6806c59 commit 5d789d7
Show file tree
Hide file tree
Showing 48 changed files with 192 additions and 188 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ bool IsLargeMemoryDevice() {
//
// Set to 4GiB, since we have 2GiB Android devices where tests flakily fail
// (e.g. Nexus 5X, crbug.com/1191195).
return base::SysInfo::AmountOfPhysicalMemory() >= 4000ULL * 1024 * 1024;
return base::SysInfo::AmountOfPhysicalMemory() >= 4000LL * 1024 * 1024;
}

bool SetAddressSpaceLimit() {
Expand Down
6 changes: 3 additions & 3 deletions base/process/process_metrics_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,11 @@ TEST_F(SystemMetricsTest, ParseMeminfo) {
EXPECT_EQ(meminfo.shmem, 140204);
EXPECT_EQ(meminfo.slab, 54212);
#endif
EXPECT_EQ(355725u,
EXPECT_EQ(355725,
base::SysInfo::AmountOfAvailablePhysicalMemory(meminfo) / 1024);
// Simulate as if there is no MemAvailable.
meminfo.available = 0;
EXPECT_EQ(374448u,
EXPECT_EQ(374448,
base::SysInfo::AmountOfAvailablePhysicalMemory(meminfo) / 1024);
meminfo = {};
EXPECT_TRUE(ParseProcMeminfo(valid_input2, &meminfo));
Expand All @@ -223,7 +223,7 @@ TEST_F(SystemMetricsTest, ParseMeminfo) {
EXPECT_EQ(meminfo.swap_total, 524280);
EXPECT_EQ(meminfo.swap_free, 524200);
EXPECT_EQ(meminfo.dirty, 4);
EXPECT_EQ(69936u,
EXPECT_EQ(69936,
base::SysInfo::AmountOfAvailablePhysicalMemory(meminfo) / 1024);
}

Expand Down
2 changes: 1 addition & 1 deletion base/profiler/stack_copier_signal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class AsyncSafeWaitableEvent {
// futex() can wake up spuriously if this memory address was previously used
// for a pthread mutex. So, also check the condition.
while (true) {
long res =
int res =
syscall(SYS_futex, futex_int_ptr(), FUTEX_WAIT | FUTEX_PRIVATE_FLAG,
0, nullptr, nullptr, 0);
if (futex_.load(std::memory_order_acquire) != 0)
Expand Down
4 changes: 2 additions & 2 deletions base/sync_socket_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "base/check_op.h"
#include "base/containers/span.h"
#include "base/files/file_util.h"
#include "base/numerics/safe_conversions.h"
#include "base/threading/scoped_blocking_call.h"
#include "build/build_config.h"

Expand Down Expand Up @@ -175,7 +174,8 @@ size_t SyncSocket::Peek() {
// If there is an error in ioctl, signal that the channel would block.
return 0;
}
return checked_cast<size_t>(number_chars);
DCHECK_GE(number_chars, 0);
return number_chars;
}

bool SyncSocket::IsValid() const {
Expand Down
17 changes: 8 additions & 9 deletions base/system/sys_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ namespace base {
namespace {
#if BUILDFLAG(IS_IOS)
// For M99, 45% of devices have 2GB of RAM, and 55% have more.
constexpr uint64_t kLowMemoryDeviceThresholdMB = 1024;
constexpr int64_t kLowMemoryDeviceThresholdMB = 1024;
#else
// Updated Desktop default threshold to match the Android 2021 definition.
constexpr uint64_t kLowMemoryDeviceThresholdMB = 2048;
constexpr int64_t kLowMemoryDeviceThresholdMB = 2048;
#endif
} // namespace

// static
uint64_t SysInfo::AmountOfPhysicalMemory() {
int64_t SysInfo::AmountOfPhysicalMemory() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableLowEndDeviceMode)) {
// Keep using 512MB as the simulated RAM amount for when users or tests have
// manually enabled low-end device mode. Note this value is different from
// the threshold used for low end devices.
constexpr uint64_t kSimulatedMemoryForEnableLowEndDeviceMode =
constexpr int64_t kSimulatedMemoryForEnableLowEndDeviceMode =
512 * 1024 * 1024;
return std::min(kSimulatedMemoryForEnableLowEndDeviceMode,
AmountOfPhysicalMemoryImpl());
Expand All @@ -47,14 +47,14 @@ uint64_t SysInfo::AmountOfPhysicalMemory() {
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemory() {
int64_t SysInfo::AmountOfAvailablePhysicalMemory() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableLowEndDeviceMode)) {
// Estimate the available memory by subtracting our memory used estimate
// from the fake |kLowMemoryDeviceThresholdMB| limit.
uint64_t memory_used =
int64_t memory_used =
AmountOfPhysicalMemoryImpl() - AmountOfAvailablePhysicalMemoryImpl();
uint64_t memory_limit = kLowMemoryDeviceThresholdMB * 1024 * 1024;
int64_t memory_limit = kLowMemoryDeviceThresholdMB * 1024 * 1024;
// std::min ensures no underflow, as |memory_used| can be > |memory_limit|.
return memory_limit - std::min(memory_used, memory_limit);
}
Expand Down Expand Up @@ -82,8 +82,7 @@ bool DetectLowEndDevice() {
return false;

int ram_size_mb = SysInfo::AmountOfPhysicalMemoryMB();
return ram_size_mb > 0 &&
static_cast<uint64_t>(ram_size_mb) <= kLowMemoryDeviceThresholdMB;
return (ram_size_mb > 0 && ram_size_mb <= kLowMemoryDeviceThresholdMB);
}

// static
Expand Down
12 changes: 6 additions & 6 deletions base/system/sys_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,19 @@ class BASE_EXPORT SysInfo {
// Return the number of bytes of physical memory on the current machine.
// If low-end device mode is manually enabled via command line flag, this
// will return the lesser of the actual physical memory, or 512MB.
static uint64_t AmountOfPhysicalMemory();
static int64_t AmountOfPhysicalMemory();

// Return the number of bytes of current available physical memory on the
// machine.
// (The amount of memory that can be allocated without any significant
// impact on the system. It can lead to freeing inactive file-backed
// and/or speculative file-backed memory).
static uint64_t AmountOfAvailablePhysicalMemory();
static int64_t AmountOfAvailablePhysicalMemory();

// Return the number of bytes of virtual memory of this process. A return
// value of zero means that there is no limit on the available virtual
// memory.
static uint64_t AmountOfVirtualMemory();
static int64_t AmountOfVirtualMemory();

// Return the number of megabytes of physical memory on the current machine.
static int AmountOfPhysicalMemoryMB() {
Expand Down Expand Up @@ -215,14 +215,14 @@ class BASE_EXPORT SysInfo {
FRIEND_TEST_ALL_PREFIXES(SysInfoTest, AmountOfAvailablePhysicalMemory);
FRIEND_TEST_ALL_PREFIXES(debug::SystemMetricsTest, ParseMeminfo);

static uint64_t AmountOfPhysicalMemoryImpl();
static uint64_t AmountOfAvailablePhysicalMemoryImpl();
static int64_t AmountOfPhysicalMemoryImpl();
static int64_t AmountOfAvailablePhysicalMemoryImpl();
static bool IsLowEndDeviceImpl();
static HardwareInfo GetHardwareInfoSync();

#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID) || \
BUILDFLAG(IS_AIX)
static uint64_t AmountOfAvailablePhysicalMemory(
static int64_t AmountOfAvailablePhysicalMemory(
const SystemMemoryInfoKB& meminfo);
#endif
};
Expand Down
6 changes: 3 additions & 3 deletions base/system/sys_info_fuchsia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ int64_t GetAmountOfTotalDiskSpaceAndVolumePath(const FilePath& path,
} // namespace

// static
uint64_t SysInfo::AmountOfPhysicalMemoryImpl() {
int64_t SysInfo::AmountOfPhysicalMemoryImpl() {
return zx_system_get_physmem();
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
int64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
// TODO(https://crbug.com/986608): Implement this.
NOTIMPLEMENTED_LOG_ONCE();
return 0;
Expand All @@ -108,7 +108,7 @@ int SysInfo::NumberOfProcessors() {
}

// static
uint64_t SysInfo::AmountOfVirtualMemory() {
int64_t SysInfo::AmountOfVirtualMemory() {
return 0;
}

Expand Down
9 changes: 4 additions & 5 deletions base/system/sys_info_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/check_op.h"
#include "base/mac/scoped_mach_port.h"
#include "base/notreached.h"
#include "base/numerics/safe_conversions.h"
#include "base/process/process_metrics.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
Expand Down Expand Up @@ -116,7 +115,7 @@
}

// static
uint64_t SysInfo::AmountOfPhysicalMemoryImpl() {
int64_t SysInfo::AmountOfPhysicalMemoryImpl() {
struct host_basic_info hostinfo;
mach_msg_type_number_t count = HOST_BASIC_INFO_COUNT;
base::mac::ScopedMachSendRight host(mach_host_self());
Expand All @@ -127,17 +126,17 @@
return 0;
}
DCHECK_EQ(HOST_BASIC_INFO_COUNT, count);
return hostinfo.max_mem;
return static_cast<int64_t>(hostinfo.max_mem);
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
int64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
SystemMemoryInfoKB info;
if (!GetSystemMemoryInfo(&info))
return 0;
// We should add inactive file-backed memory also but there is no such
// information from iOS unfortunately.
return checked_cast<uint64_t>(info.free + info.speculative) * 1024;
return static_cast<int64_t>(info.free + info.speculative) * 1024;
}

// static
Expand Down
26 changes: 14 additions & 12 deletions base/system/sys_info_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,50 +24,52 @@

namespace {

uint64_t AmountOfMemory(int pages_name) {
int64_t AmountOfMemory(int pages_name) {
long pages = sysconf(pages_name);
long page_size = sysconf(_SC_PAGESIZE);
if (pages < 0 || page_size < 0)
if (pages == -1 || page_size == -1) {
NOTREACHED();
return 0;
return static_cast<uint64_t>(pages) * static_cast<uint64_t>(page_size);
}
return static_cast<int64_t>(pages) * page_size;
}

uint64_t AmountOfPhysicalMemory() {
int64_t AmountOfPhysicalMemory() {
return AmountOfMemory(_SC_PHYS_PAGES);
}

base::LazyInstance<
base::internal::LazySysInfoValue<uint64_t, AmountOfPhysicalMemory>>::Leaky
base::internal::LazySysInfoValue<int64_t, AmountOfPhysicalMemory>>::Leaky
g_lazy_physical_memory = LAZY_INSTANCE_INITIALIZER;

} // namespace

namespace base {

// static
uint64_t SysInfo::AmountOfPhysicalMemoryImpl() {
int64_t SysInfo::AmountOfPhysicalMemoryImpl() {
return g_lazy_physical_memory.Get().value();
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
int64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
SystemMemoryInfoKB info;
if (!GetSystemMemoryInfo(&info))
return 0;
return AmountOfAvailablePhysicalMemory(info);
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemory(
int64_t SysInfo::AmountOfAvailablePhysicalMemory(
const SystemMemoryInfoKB& info) {
// See details here:
// https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773
// The fallback logic (when there is no MemAvailable) would be more precise
// if we had info about zones watermarks (/proc/zoneinfo).
int res_kb = info.available != 0
? info.available - info.active_file
: info.free + info.reclaimable + info.inactive_file;
return checked_cast<uint64_t>(res_kb) * 1024;
int64_t res_kb = info.available != 0
? info.available - info.active_file
: info.free + info.reclaimable + info.inactive_file;
return res_kb * 1024;
}

// static
Expand Down
9 changes: 4 additions & 5 deletions base/system/sys_info_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "base/mac/mac_util.h"
#include "base/mac/scoped_mach_port.h"
#include "base/notreached.h"
#include "base/numerics/safe_conversions.h"
#include "base/process/process_metrics.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
Expand Down Expand Up @@ -75,7 +74,7 @@
}

// static
uint64_t SysInfo::AmountOfPhysicalMemoryImpl() {
int64_t SysInfo::AmountOfPhysicalMemoryImpl() {
struct host_basic_info hostinfo;
mach_msg_type_number_t count = HOST_BASIC_INFO_COUNT;
base::mac::ScopedMachSendRight host(mach_host_self());
Expand All @@ -86,17 +85,17 @@
return 0;
}
DCHECK_EQ(HOST_BASIC_INFO_COUNT, count);
return hostinfo.max_mem;
return static_cast<int64_t>(hostinfo.max_mem);
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
int64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
SystemMemoryInfoKB info;
if (!GetSystemMemoryInfo(&info))
return 0;
// We should add inactive file-backed memory also but there is no such
// information from Mac OS unfortunately.
return checked_cast<uint64_t>(info.free + info.speculative) * 1024;
return static_cast<int64_t>(info.free + info.speculative) * 1024;
}

// static
Expand Down
12 changes: 7 additions & 5 deletions base/system/sys_info_openbsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@

namespace {

uint64_t AmountOfMemory(int pages_name) {
int64_t AmountOfMemory(int pages_name) {
long pages = sysconf(pages_name);
long page_size = sysconf(_SC_PAGESIZE);
if (pages < 0 || page_size < 0)
if (pages == -1 || page_size == -1) {
NOTREACHED();
return 0;
return static_cast<uint64_t>(pages) * static_cast<uint64_t>(page_size);
}
return static_cast<int64_t>(pages) * page_size;
}

} // namespace
Expand All @@ -39,12 +41,12 @@ int SysInfo::NumberOfProcessors() {
}

// static
uint64_t SysInfo::AmountOfPhysicalMemoryImpl() {
int64_t SysInfo::AmountOfPhysicalMemoryImpl() {
return AmountOfMemory(_SC_PHYS_PAGES);
}

// static
uint64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
int64_t SysInfo::AmountOfAvailablePhysicalMemoryImpl() {
// We should add inactive file-backed memory also but there is no such
// information from OpenBSD unfortunately.
return AmountOfMemory(_SC_AVPHYS_PAGES);
Expand Down
Loading

0 comments on commit 5d789d7

Please sign in to comment.