V8 crashes when profile generator is trying to allocate a raw_entries_ buffer more than 2Gb.

V8 crashes when profile generator allocates a buffer for HeapEntry'es bigger than 2Gb.

BUG=none
TEST=profile-generator tests

[mnaganov (inactive), 2012-03-08 15:25:10]

Please brush up issues with types used.

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1102: return (uint64_t)sizeof(HeapEntry) *
entries_count         // NOLINT
Please, use static_cast.

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1135: char* FirstAddress(size_t minimum_size) {
I think, you should use Address type.

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1140: ASSERT(raw_data_.length());
!= 0

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1141: if (next_entry_candidate - last_chunk_ +
minimum_size > last_chunk_size_) {
Note: Please verify that expressions like these compile both on ia32, x64 and on
Windows.

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:3475: "Maximum snapshot size is %d MB. "
Is %d the correct type?

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.h
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.h...
src/profile-generator.h:651: uint64_t raw_entries_size() { return
raw_entries_size_; }
Why not size_t?

[alexeif, 2012-03-10 10:32:02]

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1102: return (uint64_t)sizeof(HeapEntry) *
entries_count         // NOLINT
There's no need to use casts here. sizeof has the type of size_t which has the
width of the pointer type. Just change the return type of EntriesSize to size_t
and that should be enough.

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1151: last_chunk_ = NewArray<char>(last_chunk_size_);
Can't we just change the NewArray argument to size_t instead of workarounding it
with such a kludge?

https://chromiumcodereview.appspot.com/9632020/diff/1/src/profile-generator.c...
src/profile-generator.cc:3476: "Actual snapshot size is %u MB.",
if raw_entries_size() is uint64_t then it should be %llu. If you change it to
size_t then use %lu.

[loislo, 2012-03-11 10:14:13]

alexeif comments addressed

[alexeif, 2012-03-11 11:15:59]

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
src/profile-generator.cc:1101: int retainers_count) {
tabs?

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
src/profile-generator.cc:3440: "Maximum snapshot size is %d MB. "
both arguments have type of size_t which is 64-bit on x64, so %d won't work. %lu
is the right specifier here.

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
src/profile-generator.cc:3441: "Actual snapshot size is %d MB.",
ditto

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generator.h
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
src/profile-generator.h:580: int EntrySize() { return EntriesSize(1,
children_count_, retainers_count_); }
size_t instead of int.
Btw it should be giving narrowing type cast warning on x64.

https://chromiumcodereview.appspot.com/9632020/diff/5001/src/profile-generato...
src/profile-generator.h:589: int retainers_count);
nit: I'm not sure tabs are allowed.

[alexeif, 2012-03-11 14:30:04]

https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generato...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generato...
src/profile-generator.cc:1124: static_cast<uint64_t>(6000) * MB;
nit: V8_UINT64_C(6000)

[loislo, 2012-03-11 14:38:05]

On 2012/03/11 14:30:04, alexeif wrote:
>
https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generato...
> File src/profile-generator.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generato...
> src/profile-generator.cc:1124: static_cast<uint64_t>(6000) * MB;
> nit: V8_UINT64_C(6000)

There is no such macros for ia32-linux and ia32-win32

[mnaganov (inactive), 2012-03-11 14:43:57]

LGTM

https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generator.h
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/9632020/diff/5005/src/profile-generato...
src/profile-generator.h:580: size_t EntrySize() { return EntriesSize(1,
children_count_, retainers_count_); }
nit: 80 chars