New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 637279 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue v8:5267
issue 636331



Sign in to add a comment

Context null when entering runtime from TF code

Project Member Reported by mlippautz@chromium.org, Aug 12 2016

Issue description

While trying to land https://codereview.chromium.org/2232653003/ I got blocked by gbemu failing. Turns out the context is null at some point. We enter the runtime from a turbofan'ed function. Attached is jco of the TF function.

Repro in [1]. Works even with v8_optimized_debug = false (even though it takes a couple tries).

I ran my CL within Chrome on the real web, so I am pretty sure it works in general. Will continue investigating next week.

(gdb) bt
#0  0x00005555559b08db in v8::internal::FixedArray::get (this=0x0, index=3) at ../../src/objects-inl.h:2334
#1  0x00005555559b05d4 in v8::internal::Context::native_context (this=0x0) at ../../src/contexts-inl.h:71
#2  0x00005555561e4895 in v8::internal::Map::TransitionElementsTo (map=..., to_kind=v8::internal::FAST_ELEMENTS)
    at ../../src/objects.cc:5174
#3  0x00005555561e52a3 in v8::internal::JSObject::GetElementsTransitionMap (object=...,
    to_kind=v8::internal::FAST_ELEMENTS) at ../../src/objects.cc:5237
#4  0x00005555561e79b3 in v8::internal::JSObject::TransitionElementsKind (object=...,
    to_kind=v8::internal::FAST_ELEMENTS) at ../../src/objects.cc:15526
#5  0x000055555620dfc9 in v8::internal::AllocationSite::DigestTransitionFeedback (site=...,
    to_kind=v8::internal::FAST_ELEMENTS) at ../../src/objects.cc:15444
#6  0x000055555620e2c2 in v8::internal::JSObject::UpdateAllocationSite (object=...,
    to_kind=v8::internal::FAST_ELEMENTS) at ../../src/objects.cc:15501
#7  0x0000555555f88959 in v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedObjectElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)2> >::BasicGrowCapacityAndConvertImpl (object=..., old_elements=..., from_kind=v8::internal::FAST_ELEMENTS,
    to_kind=v8::internal::FAST_ELEMENTS, capacity=76300) at ../../src/elements.cc:877
#8  0x0000555555f8889a in v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedObjectElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)2> >::GrowCapacityAndConvertImpl (object=..., capacity=76300) at ../../src/elements.cc:862
#9  0x0000555555f874b7 in v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedObjectElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)2> >::GrowCapacityAndConvert (this=0x555556cc13a0, object=..., capacity=76300) at ../../src/elements.cc:891
#10 0x000055555633af5d in v8::internal::__RT_impl_Runtime_GrowArrayElements (args=..., isolate=0x555556cc3000)
    at ../../src/runtime/runtime-array.cc:385
#11 0x000055555633ac19 in v8::internal::Runtime_GrowArrayElements (args_length=2, args_object=0x7fffffffc6a0,
    isolate=0x555556cc3000) at ../../src/runtime/runtime-array.cc:363
#12 0x00001d8d02d043a7 in ?? () <-- CEntryStub -->
#13 0x00001d8d02d042e1 in ?? () <-- CEntryStub -->
#14 0x00007fffffffc670 in ?? ()
#15 0x0000000300000000 in ?? ()
#16 0x00007fffffffc6e0 in ?? ()
#17 0x00001d8d02f7b933 in ?? () <-- TURBOFAN'ed -->
#18 0x0000c6a700000000 in ?? ()
#19 0x000013e97ad45101 in ?? ()
#20 0x0000000000001a0c in ?? ()
#21 0x000013e97ad45101 in ?? ()
#22 0x0000000000000000 in ?? ()

[1]: https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20-%20debug%20-%20avx2/builds/9173/steps/Bisect%2020e2ea80.Retry/logs/gbemu-part1

 
optcode.txt
122 KB View Download
Blocking: 636331
Cc: hpayer@chromium.org
Summary: Context null when entering runtime from TF code (was: Context null when entering from TF code)
Blocking: v8:5267
Cc: mvstan...@chromium.org
Components: Blink>JavaScript>Compiler
Labels: OS-All
Owner: cbruni@chromium.org
Status: Assigned (was: Untriaged)
[Side note: If you happen to hit a V8 bug, using the V8 bug tracker is more convenient; there we have dedicated labels for TurboFan issues, etc. which makes it a bit easier to track]

The problem here is that %GrowArrayElements tries to be too smart for us. What the optimizing compilers (this applies to both TurboFan and Crankshaft) actually need is just a stupid function that tries to grow the elements backing store to a certain size, or fail (signaled by returning Smi 0) if it cannot because it would have to normalize and thereby change maps. So basically we need some kind of MaybeGrowFastElements in the elements accessors that does this, and since that shouldn't try to be smart, but just fail if it cannot handle the case, it doesn't need access to a (native) context. In Crankshaft this was semi-okish, since it always passed some context (even tho that doesn't necessarily corresponded to the creation context of the receiver), but TurboFan doesn't pass a context here on purpose, as the expected operation is inherently context independent.

I'm assigning this to Camillo. Feel free to ask compiler folks for help (not sure how urgent this is for the GC folks).

Thanks for the analysis! Saved me quiet some time. Will file future issues on the V8 tracker.

Weirdly enough: This is kind of urgent, but only because it's reliably triggering on gbemu and afaik we don't disable benchmarks on the waterfall. The CL that it's blocking is an enabler for further optimizations.

Comment 5 by cbruni@chromium.org, Aug 16 2016

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a9fd19f4d137a8a2f205288afd348a03255fa685

commit a9fd19f4d137a8a2f205288afd348a03255fa685
Author: jkummerow <jkummerow@chromium.org>
Date: Thu Aug 25 11:39:27 2016

[elements, turbofan] Implement simple GrowElements

Unlike Crankshaft, Turbofan does not provide a context when trying to grow
elements. Depending on the code path we might end up updating transitioning
elements kinds in allocation sites for which we need access to the current
context. Unlike GrowCapacityAndConvert, the newly introduced GrowCapacity simply
returns false in cases where map transitions are involved.

BUG= chromium:637279 

Patch by Camillo Bruni <cbruni@chromium.org>,
originally reviewed at https://codereview.chromium.org/2244983004/

Review-Url: https://codereview.chromium.org/2252393002
Cr-Commit-Position: refs/heads/master@{#38901}

[modify] https://crrev.com/a9fd19f4d137a8a2f205288afd348a03255fa685/src/elements.cc
[modify] https://crrev.com/a9fd19f4d137a8a2f205288afd348a03255fa685/src/elements.h
[modify] https://crrev.com/a9fd19f4d137a8a2f205288afd348a03255fa685/src/objects.cc
[modify] https://crrev.com/a9fd19f4d137a8a2f205288afd348a03255fa685/src/objects.h
[modify] https://crrev.com/a9fd19f4d137a8a2f205288afd348a03255fa685/src/runtime/runtime-array.cc

Status: Fixed (was: Started)
Thanks Jakob!

Sign in to add a comment