Commit 8678fc62 authored by Kim-Anh Tran's avatar Kim-Anh Tran Committed by V8 LUCI CQ

[heap-snapshot] Declaring progress counter as uint32_t (instead of int)

A test was overflowing on the progress counter when using int as type.
This CL is fixing the progress counter to use uint32_t, and re-enables
the test.

Why uint32_t instead of size_t?
In the referenced bug, the progress_counter_ (but not the
progress_total_) triggered an overflow; and since these two counters
should be relatively similar (the total count is an estimate, and can
be less than the actual progress count), we do not expect the
count to increase much more than we can already encode with int.


Bug: chromium:1246860
Change-Id: I9769884ef60d352b3787c2223e528ddf33b0b23e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3245116
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77551}
parent 1e0567fb
......@@ -603,7 +603,7 @@ class V8_EXPORT ActivityControl {
* Notify about current progress. The activity can be stopped by
* returning kAbort as the callback result.
*/
virtual ControlOption ReportProgressValue(int done, int total) = 0;
virtual ControlOption ReportProgressValue(uint32_t done, uint32_t total) = 0;
};
/**
......
......@@ -35,7 +35,7 @@ class HeapSnapshotProgress final : public v8::ActivityControl {
public:
explicit HeapSnapshotProgress(protocol::HeapProfiler::Frontend* frontend)
: m_frontend(frontend) {}
ControlOption ReportProgressValue(int done, int total) override {
ControlOption ReportProgressValue(uint32_t done, uint32_t total) override {
m_frontend->reportHeapSnapshotProgress(done, total,
protocol::Maybe<bool>());
if (done >= total) {
......
......@@ -570,8 +570,8 @@ class HeapSnapshotGenerator : public SnapshottingProgressReportingInterface {
HeapEntriesMap entries_map_;
SmiEntriesMap smis_map_;
// Used during snapshot generation.
int progress_counter_;
int progress_total_;
uint32_t progress_counter_;
uint32_t progress_total_;
Heap* heap_;
};
......
......@@ -1556,7 +1556,7 @@ class TestActivityControl : public v8::ActivityControl {
total_(0),
abort_count_(abort_count),
reported_finish_(false) {}
ControlOption ReportProgressValue(int done, int total) override {
ControlOption ReportProgressValue(uint32_t done, uint32_t total) override {
done_ = done;
total_ = total;
CHECK_LE(done_, total_);
......
......@@ -414,7 +414,6 @@
'debugger/wasm-stepping-with-source-map': [SKIP],
'debugger/wasm-unnamed-function-names': [SKIP],
'heap-profiler/collect-garbage': [SKIP],
'heap-profiler/console-retaining-path': [SKIP],
'heap-profiler/sampling-heap-profiler': [SKIP],
'heap-profiler/take-heap-snapshot-on-pause': [SKIP],
'json-parse': [SKIP],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment