Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Issue 18277006: Bugfix: array constructor stub was missing a case (Closed)

Created:
7 years, 5 months ago by mvstanton
Modified:
7 years, 5 months ago
CC:
v8-dev
Visibility:
Public.

Description

Bugfix: The general array constructor stub did not handle the case properly when it is called with a function pointer in the type cell, instead assuming that an AllocationSite object should be present. The case where this can happen is if the cell is uninitialized, then the first constructor call made is to the Array function of a different context. In that case, we'll store the function pointer in the cell, and then go ahead and call the array constructor stub too. The bug is fixed by checking for the AllocationSite object map. If not found, the constructor stub goes forward with a default ElementsKind, just as in several other cases. A test in allocation-site-info.js was beefed up to make sure the state chain described above is traversed. BUG= R=hpayer@chromium.org, hpayer@google.com Committed: https://code.google.com/p/v8/source/detail?r=15555

Patch Set 1 #

Patch Set 2 : Incorrectly commented out test code #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -20 lines) Patch
M src/arm/code-stubs-arm.cc View 1 chunk +4 lines, -7 lines 0 comments Download
M src/ia32/code-stubs-ia32.cc View 1 chunk +4 lines, -7 lines 0 comments Download
M src/x64/code-stubs-x64.cc View 1 chunk +4 lines, -6 lines 0 comments Download
M test/mjsunit/allocation-site-info.js View 1 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
mvstanton
Here is the change we discussed. ArrayConstructorStub::Generate was not handling the case where the type ...
7 years, 5 months ago (2013-07-08 14:32:14 UTC) #1
payer
LGTM
7 years, 5 months ago (2013-07-08 14:37:47 UTC) #2
Hannes Payer (out of office)
lgtm
7 years, 5 months ago (2013-07-08 14:39:19 UTC) #3
mvstanton
7 years, 5 months ago (2013-07-08 14:42:02 UTC) #4
Message was sent while issue was closed.
Committed patchset #2 manually as r15555 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698