Yes, it is worrisome that the test is so brittle - it has to trigger GC on
heap
number allocation. In fact, the test case only repros for x64, so we do not
have
full test coverage here. I did some sanity checks by simulating the GC
manually
(simply by jumping to the gc_cleanup label rather than calling
AllocateHeapNumber), and then checking in the debugger that we fill the
array
with the expected value.
On the plus side, it should be safe for release mode because the code only
affects heap verification mode.
https://codereview.chromium.org/228643002/diff/1/src/arm/codegen-arm.cc
File src/arm/codegen-arm.cc (right):
https://codereview.chromium.org/228643002/diff/1/src/arm/codegen-arm.cc#newcode610
src/arm/codegen-arm.cc:610: __ mov(r0, Operand(Smi::FromInt(0)));
On 2014/04/08 14:19:27, Hannes Payer wrote:
In allocation folding, we store the one_pointer_filler_map to
uninitialized
memory when we verify the heap. Either do that to stay in sync with
allocation
folding implementation or install a free_space_map with the given
size. I
actually would prefer option B. Then you do not have to loop over the
array.
Replaced with one_pointer_filler_map (although I am still not completely
sure what is the advantage over Smi 0 - the array is dead, so any valid
value should be fine).
As for option B - we would have to calculate the size again and that
seems to be more error prone than just using the counter in r3 to finish
the initialization of the array. Performance does not matter here. The
case is rare and we are verifying heap - that dominates the running time
anyway.
https://codereview.chromium.org/228643002/diff/1/test/mjsunit/regress/regress-transition-elements-heap-verification.js
File
test/mjsunit/regress/regress-transition-elements-heap-verification.js
(right):
https://codereview.chromium.org/228643002/diff/1/test/mjsunit/regress/regress-transition-elements-heap-verification.js#newcode7
test/mjsunit/regress/regress-transition-elements-heap-verification.js:7:
%SetAllocationTimeout(1000000, 1000000);
On 2014/04/08 14:19:27, Hannes Payer wrote:
The test looks super fragile, but there is probably nothing we can do
here.
Indeed, I am not sure how to make it better (it took embarrassingly long
time to minimize the original ClusterFuzz test case to the somewhat
manageable repro here).
https://codereview.chromium.org/228643002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
