Skip to content

Commit 46c4979

Browse files
bnoordhuisCommit Bot
bnoordhuis
authored and
Commit Bot
committed
Use wider types for max_old_space_size and co.
Make --max_old_space_size and friends work with values >= 2**31. Such values did not work reliably (or sometimes not all) due to signed integer overflow in size computations, which is UB. Fixes nodejs/node#18786. Bug: chromium:814138 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182 Reviewed-on: https://chromium-review.googlesource.com/927063 Commit-Queue: Ben Noordhuis <[email protected]> Reviewed-by: Michael Lippautz <[email protected]> Cr-Commit-Position: refs/heads/master@{#51433}
1 parent 4724d0f commit 46c4979

6 files changed

Lines changed: 89 additions & 53 deletions

File tree

‎include/v8.h‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6087,13 +6087,13 @@ class V8_EXPORT ResourceConstraints {
60876087

60886088
// Returns the max semi-space size in MB.
60896089
V8_DEPRECATE_SOON("Use max_semi_space_size_in_kb()",
6090-
int max_semi_space_size()) {
6091-
return static_cast<int>(max_semi_space_size_in_kb_ / 1024);
6090+
size_t max_semi_space_size()) {
6091+
return max_semi_space_size_in_kb_ / 1024;
60926092
}
60936093

60946094
// Sets the max semi-space size in MB.
60956095
V8_DEPRECATE_SOON("Use set_max_semi_space_size_in_kb(size_t limit_in_kb)",
6096-
void set_max_semi_space_size(int limit_in_mb)) {
6096+
void set_max_semi_space_size(size_t limit_in_mb)) {
60976097
max_semi_space_size_in_kb_ = limit_in_mb * 1024;
60986098
}
60996099

@@ -6107,16 +6107,16 @@ class V8_EXPORT ResourceConstraints {
61076107
max_semi_space_size_in_kb_ = limit_in_kb;
61086108
}
61096109

6110-
int max_old_space_size() const { return max_old_space_size_; }
6111-
void set_max_old_space_size(int limit_in_mb) {
6110+
size_t max_old_space_size() const { return max_old_space_size_; }
6111+
void set_max_old_space_size(size_t limit_in_mb) {
61126112
max_old_space_size_ = limit_in_mb;
61136113
}
61146114
V8_DEPRECATE_SOON("max_executable_size_ is subsumed by max_old_space_size_",
6115-
int max_executable_size() const) {
6115+
size_t max_executable_size() const) {
61166116
return max_executable_size_;
61176117
}
61186118
V8_DEPRECATE_SOON("max_executable_size_ is subsumed by max_old_space_size_",
6119-
void set_max_executable_size(int limit_in_mb)) {
6119+
void set_max_executable_size(size_t limit_in_mb)) {
61206120
max_executable_size_ = limit_in_mb;
61216121
}
61226122
uint32_t* stack_limit() const { return stack_limit_; }
@@ -6127,17 +6127,15 @@ class V8_EXPORT ResourceConstraints {
61276127
code_range_size_ = limit_in_mb;
61286128
}
61296129
size_t max_zone_pool_size() const { return max_zone_pool_size_; }
6130-
void set_max_zone_pool_size(const size_t bytes) {
6131-
max_zone_pool_size_ = bytes;
6132-
}
6130+
void set_max_zone_pool_size(size_t bytes) { max_zone_pool_size_ = bytes; }
61336131

61346132
private:
61356133
// max_semi_space_size_ is in KB
61366134
size_t max_semi_space_size_in_kb_;
61376135

61386136
// The remaining limits are in MB
6139-
int max_old_space_size_;
6140-
int max_executable_size_;
6137+
size_t max_old_space_size_;
6138+
size_t max_executable_size_;
61416139
uint32_t* stack_limit_;
61426140
size_t code_range_size_;
61436141
size_t max_zone_pool_size_;

‎src/api.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -934,8 +934,7 @@ void ResourceConstraints::ConfigureDefaults(uint64_t physical_memory,
934934
uint64_t virtual_memory_limit) {
935935
set_max_semi_space_size_in_kb(
936936
i::Heap::ComputeMaxSemiSpaceSize(physical_memory));
937-
set_max_old_space_size(
938-
static_cast<int>(i::Heap::ComputeMaxOldGenerationSize(physical_memory)));
937+
set_max_old_space_size(i::Heap::ComputeMaxOldGenerationSize(physical_memory));
939938
set_max_zone_pool_size(i::AccountingAllocator::kMaxPoolSize);
940939

941940
if (virtual_memory_limit > 0 && i::kRequiresCodeRange) {
@@ -950,7 +949,7 @@ void ResourceConstraints::ConfigureDefaults(uint64_t physical_memory,
950949
void SetResourceConstraints(i::Isolate* isolate,
951950
const ResourceConstraints& constraints) {
952951
size_t semi_space_size = constraints.max_semi_space_size_in_kb();
953-
int old_space_size = constraints.max_old_space_size();
952+
size_t old_space_size = constraints.max_old_space_size();
954953
size_t code_range_size = constraints.code_range_size();
955954
size_t max_pool_size = constraints.max_zone_pool_size();
956955
if (semi_space_size != 0 || old_space_size != 0 || code_range_size != 0) {

‎src/flag-definitions.h‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,15 @@ struct MaybeBoolFlag {
163163
#define DEFINE_INT(nam, def, cmt) FLAG(INT, int, nam, def, cmt)
164164
#define DEFINE_UINT(nam, def, cmt) FLAG(UINT, unsigned int, nam, def, cmt)
165165
#define DEFINE_FLOAT(nam, def, cmt) FLAG(FLOAT, double, nam, def, cmt)
166+
#define DEFINE_SIZE_T(nam, def, cmt) FLAG(SIZE_T, size_t, nam, def, cmt)
166167
#define DEFINE_STRING(nam, def, cmt) FLAG(STRING, const char*, nam, def, cmt)
167168
#define DEFINE_ARGS(nam, cmt) \
168169
FLAG(ARGS, JSArguments, nam, {0 COMMA nullptr}, cmt)
169170

170171
#define DEFINE_ALIAS_BOOL(alias, nam) FLAG_ALIAS(BOOL, bool, alias, nam)
171172
#define DEFINE_ALIAS_INT(alias, nam) FLAG_ALIAS(INT, int, alias, nam)
172173
#define DEFINE_ALIAS_FLOAT(alias, nam) FLAG_ALIAS(FLOAT, double, alias, nam)
174+
#define DEFINE_ALIAS_SIZE_T(alias, nam) FLAG_ALIAS(SIZE_T, size_t, alias, nam)
173175
#define DEFINE_ALIAS_STRING(alias, nam) \
174176
FLAG_ALIAS(STRING, const char*, alias, nam)
175177
#define DEFINE_ALIAS_ARGS(alias, nam) FLAG_ALIAS(ARGS, JSArguments, alias, nam)
@@ -622,18 +624,18 @@ DEFINE_INT(stress_sampling_allocation_profiler, 0,
622624
"Enables sampling allocation profiler with X as a sample interval")
623625

624626
// Garbage collections flags.
625-
DEFINE_INT(min_semi_space_size, 0,
626-
"min size of a semi-space (in MBytes), the new space consists of two"
627-
"semi-spaces")
628-
DEFINE_INT(max_semi_space_size, 0,
629-
"max size of a semi-space (in MBytes), the new space consists of two"
630-
"semi-spaces")
627+
DEFINE_SIZE_T(min_semi_space_size, 0,
628+
"min size of a semi-space (in MBytes), the new space consists of "
629+
"two semi-spaces")
630+
DEFINE_SIZE_T(max_semi_space_size, 0,
631+
"max size of a semi-space (in MBytes), the new space consists of "
632+
"two semi-spaces")
631633
DEFINE_INT(semi_space_growth_factor, 2, "factor by which to grow the new space")
632634
DEFINE_BOOL(experimental_new_space_growth_heuristic, false,
633635
"Grow the new space based on the percentage of survivors instead "
634636
"of their absolute value.")
635-
DEFINE_INT(max_old_space_size, 0, "max size of the old space (in Mbytes)")
636-
DEFINE_INT(initial_old_space_size, 0, "initial old space size (in Mbytes)")
637+
DEFINE_SIZE_T(max_old_space_size, 0, "max size of the old space (in Mbytes)")
638+
DEFINE_SIZE_T(initial_old_space_size, 0, "initial old space size (in Mbytes)")
637639
DEFINE_BOOL(gc_global, false, "always perform global GCs")
638640
DEFINE_INT(random_gc_interval, 0,
639641
"Collect garbage after random(0, X) allocations. It overrides "

‎src/flags.cc‎

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "src/flags.h"
66

77
#include <cctype>
8+
#include <cerrno>
89
#include <cstdlib>
910
#include <sstream>
1011

@@ -39,6 +40,7 @@ struct Flag {
3940
TYPE_INT,
4041
TYPE_UINT,
4142
TYPE_FLOAT,
43+
TYPE_SIZE_T,
4244
TYPE_STRING,
4345
TYPE_ARGS
4446
};
@@ -81,6 +83,11 @@ struct Flag {
8183
return reinterpret_cast<double*>(valptr_);
8284
}
8385

86+
size_t* size_t_variable() const {
87+
DCHECK(type_ == TYPE_SIZE_T);
88+
return reinterpret_cast<size_t*>(valptr_);
89+
}
90+
8491
const char* string_value() const {
8592
DCHECK(type_ == TYPE_STRING);
8693
return *reinterpret_cast<const char**>(valptr_);
@@ -119,6 +126,11 @@ struct Flag {
119126
return *reinterpret_cast<const double*>(defptr_);
120127
}
121128

129+
size_t size_t_default() const {
130+
DCHECK(type_ == TYPE_SIZE_T);
131+
return *reinterpret_cast<const size_t*>(defptr_);
132+
}
133+
122134
const char* string_default() const {
123135
DCHECK(type_ == TYPE_STRING);
124136
return *reinterpret_cast<const char* const *>(defptr_);
@@ -142,6 +154,8 @@ struct Flag {
142154
return *uint_variable() == uint_default();
143155
case TYPE_FLOAT:
144156
return *float_variable() == float_default();
157+
case TYPE_SIZE_T:
158+
return *size_t_variable() == size_t_default();
145159
case TYPE_STRING: {
146160
const char* str1 = string_value();
147161
const char* str2 = string_default();
@@ -173,6 +187,9 @@ struct Flag {
173187
case TYPE_FLOAT:
174188
*float_variable() = float_default();
175189
break;
190+
case TYPE_SIZE_T:
191+
*size_t_variable() = size_t_default();
192+
break;
176193
case TYPE_STRING:
177194
set_string_value(string_default(), false);
178195
break;
@@ -201,6 +218,8 @@ static const char* Type2String(Flag::FlagType type) {
201218
case Flag::TYPE_UINT:
202219
return "uint";
203220
case Flag::TYPE_FLOAT: return "float";
221+
case Flag::TYPE_SIZE_T:
222+
return "size_t";
204223
case Flag::TYPE_STRING: return "string";
205224
case Flag::TYPE_ARGS: return "arguments";
206225
}
@@ -227,6 +246,9 @@ std::ostream& operator<<(std::ostream& os, const Flag& flag) { // NOLINT
227246
case Flag::TYPE_FLOAT:
228247
os << *flag.float_variable();
229248
break;
249+
case Flag::TYPE_SIZE_T:
250+
os << *flag.size_t_variable();
251+
break;
230252
case Flag::TYPE_STRING: {
231253
const char* str = flag.string_value();
232254
os << (str ? str : "nullptr");
@@ -358,6 +380,27 @@ static Flag* FindFlag(const char* name) {
358380
return nullptr;
359381
}
360382

383+
template <typename T>
384+
bool TryParseUnsigned(Flag* flag, const char* arg, const char* value,
385+
char** endp, T* out_val) {
386+
// We do not use strtoul because it accepts negative numbers.
387+
// Rejects values >= 2**63 when T is 64 bits wide but that
388+
// seems like an acceptable trade-off.
389+
uint64_t max = static_cast<uint64_t>(std::numeric_limits<T>::max());
390+
errno = 0;
391+
int64_t val = static_cast<int64_t>(strtoll(value, endp, 10));
392+
if (val < 0 || static_cast<uint64_t>(val) > max || errno != 0) {
393+
PrintF(stderr,
394+
"Error: Value for flag %s of type %s is out of bounds "
395+
"[0-%" PRIu64
396+
"]\n"
397+
"Try --help for options\n",
398+
arg, Type2String(flag->type()), max);
399+
return false;
400+
}
401+
*out_val = static_cast<T>(val);
402+
return true;
403+
}
361404

362405
// static
363406
int FlagList::SetFlagsFromCommandLine(int* argc,
@@ -422,27 +465,21 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
422465
case Flag::TYPE_INT:
423466
*flag->int_variable() = static_cast<int>(strtol(value, &endp, 10));
424467
break;
425-
case Flag::TYPE_UINT: {
426-
// We do not use strtoul because it accepts negative numbers.
427-
int64_t val = static_cast<int64_t>(strtoll(value, &endp, 10));
428-
if (val < 0 || val > std::numeric_limits<unsigned int>::max()) {
429-
PrintF(stderr,
430-
"Error: Value for flag %s of type %s is out of bounds "
431-
"[0-%" PRIu64
432-
"]\n"
433-
"Try --help for options\n",
434-
arg, Type2String(flag->type()),
435-
static_cast<uint64_t>(
436-
std::numeric_limits<unsigned int>::max()));
468+
case Flag::TYPE_UINT:
469+
if (!TryParseUnsigned(flag, arg, value, &endp,
470+
flag->uint_variable())) {
437471
return_code = j;
438-
break;
439472
}
440-
*flag->uint_variable() = static_cast<unsigned int>(val);
441473
break;
442-
}
443474
case Flag::TYPE_FLOAT:
444475
*flag->float_variable() = strtod(value, &endp);
445476
break;
477+
case Flag::TYPE_SIZE_T:
478+
if (!TryParseUnsigned(flag, arg, value, &endp,
479+
flag->size_t_variable())) {
480+
return_code = j;
481+
}
482+
break;
446483
case Flag::TYPE_STRING:
447484
flag->set_string_value(value ? StrDup(value) : nullptr, true);
448485
break;

‎src/heap/heap.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5144,8 +5144,8 @@ bool Heap::ConfigureHeap(size_t max_semi_space_size_in_kb,
51445144

51455145
// The new space size must be a power of two to support single-bit testing
51465146
// for containment.
5147-
max_semi_space_size_ = base::bits::RoundUpToPowerOfTwo32(
5148-
static_cast<uint32_t>(max_semi_space_size_));
5147+
max_semi_space_size_ = static_cast<size_t>(base::bits::RoundUpToPowerOfTwo64(
5148+
static_cast<uint64_t>(max_semi_space_size_)));
51495149

51505150
if (max_semi_space_size_ == kMaxSemiSpaceSizeInKB * KB) {
51515151
// Start with at least 1*MB semi-space on machines with a lot of memory.

‎src/heap/heap.h‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -612,15 +612,15 @@ class Heap {
612612
#endif
613613

614614
// Semi-space size needs to be a multiple of page size.
615-
static const int kMinSemiSpaceSizeInKB =
615+
static const size_t kMinSemiSpaceSizeInKB =
616616
1 * kPointerMultiplier * ((1 << kPageSizeBits) / KB);
617-
static const int kMaxSemiSpaceSizeInKB =
617+
static const size_t kMaxSemiSpaceSizeInKB =
618618
16 * kPointerMultiplier * ((1 << kPageSizeBits) / KB);
619619

620620
// The old space size has to be a multiple of Page::kPageSize.
621621
// Sizes are in MB.
622-
static const int kMinOldGenerationSize = 128 * kPointerMultiplier;
623-
static const int kMaxOldGenerationSize = 1024 * kPointerMultiplier;
622+
static const size_t kMinOldGenerationSize = 128 * kPointerMultiplier;
623+
static const size_t kMaxOldGenerationSize = 1024 * kPointerMultiplier;
624624

625625
static const int kTraceRingBufferSize = 512;
626626
static const int kStacktraceBufferSize = 512;
@@ -1371,10 +1371,10 @@ class Heap {
13711371
size_t MaxOldGenerationSize() { return max_old_generation_size_; }
13721372

13731373
static size_t ComputeMaxOldGenerationSize(uint64_t physical_memory) {
1374-
const int old_space_physical_memory_factor = 4;
1375-
int computed_size =
1376-
static_cast<int>(physical_memory / i::MB /
1377-
old_space_physical_memory_factor * kPointerMultiplier);
1374+
const size_t old_space_physical_memory_factor = 4;
1375+
size_t computed_size = static_cast<size_t>(
1376+
physical_memory / i::MB / old_space_physical_memory_factor *
1377+
kPointerMultiplier);
13781378
return Max(Min(computed_size, kMaxOldGenerationSize),
13791379
kMinOldGenerationSize);
13801380
}
@@ -1386,11 +1386,11 @@ class Heap {
13861386
uint64_t capped_physical_memory =
13871387
Max(Min(physical_memory, max_physical_memory), min_physical_memory);
13881388
// linearly scale max semi-space size: (X-A)/(B-A)*(D-C)+C
1389-
int semi_space_size_in_kb =
1390-
static_cast<int>(((capped_physical_memory - min_physical_memory) *
1391-
(kMaxSemiSpaceSizeInKB - kMinSemiSpaceSizeInKB)) /
1392-
(max_physical_memory - min_physical_memory) +
1393-
kMinSemiSpaceSizeInKB);
1389+
size_t semi_space_size_in_kb =
1390+
static_cast<size_t>(((capped_physical_memory - min_physical_memory) *
1391+
(kMaxSemiSpaceSizeInKB - kMinSemiSpaceSizeInKB)) /
1392+
(max_physical_memory - min_physical_memory) +
1393+
kMinSemiSpaceSizeInKB);
13941394
return RoundUp(semi_space_size_in_kb, (1 << kPageSizeBits) / KB);
13951395
}
13961396

0 commit comments

Comments
 (0)