Bug 191468

Summary:

LLInt VectorSizeOffset should be based on offset extraction

Product:

WebKit

Reporter:

Keith Miller <keith_miller>

Component:

New Bugs

Assignee:

Keith Miller <keith_miller>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, dbates, ews-watchlist, mark.lam, msaboff, rniwa, saam, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
Archive of layout-test-results from ews102 for mac-sierra	none
Archive of layout-test-results from ews113 for mac-sierra	none
Archive of layout-test-results from ews122 for ios-simulator-wk2	none
Patch	ysuzuki: review+

Keith Miller

Reported 2018-11-09 06:45:10 PST

LLInt VectorSizeOffset should be based on PtrSize

Attachments
Patch (1.97 KB, patch) 2018-11-09 06:45 PST, Keith Miller	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (1.52 MB, application/zip) 2018-11-09 07:15 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews102 for mac-sierra (527.55 KB, application/zip) 2018-11-09 07:29 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews113 for mac-sierra (1.54 MB, application/zip) 2018-11-09 07:59 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (780.71 KB, application/zip) 2018-11-09 08:03 PST, EWS Watchlist	no flags	Details
Patch (4.96 KB, patch) 2018-11-09 08:22 PST, Keith Miller	ysuzuki: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Keith Miller

Comment 1 2018-11-09 06:45:31 PST

Created attachment 354337 [details] Patch

EWS Watchlist

Comment 2 2018-11-09 07:15:10 PST

Comment on attachment 354337 [details] Patch Attachment 354337 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9923489 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 3 2018-11-09 07:15:11 PST

Created attachment 354339 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Yusuke Suzuki

Comment 4 2018-11-09 07:22:16 PST

Comment on attachment 354337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354337&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:524 > +const VectorSizeOffset = PtrSize * 2 I think the size offset is `PtrSize + 4`, 4 for `unsigned` capacity. How about using Vector<XXX>::m_size instead?

Yusuke Suzuki

Comment 5 2018-11-09 07:27:12 PST

Comment on attachment 354337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354337&action=review >> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:524 >> +const VectorSizeOffset = PtrSize * 2 > > I think the size offset is `PtrSize + 4`, 4 for `unsigned` capacity. > How about using Vector<XXX>::m_size instead? BTW, VectorBufferOffset is wrongly used in some places of LLInt. It is also used for RefCountedArray (m_argumentValueProfiles). I think we should use RefCountedArray<XXX>::m_data instead.

EWS Watchlist

Comment 6 2018-11-09 07:29:53 PST

Comment on attachment 354337 [details] Patch Attachment 354337 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/9923694 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 7 2018-11-09 07:29:54 PST

Created attachment 354342 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6

Keith Miller

Comment 8 2018-11-09 07:36:43 PST

Comment on attachment 354337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354337&action=review >>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:524 >>> +const VectorSizeOffset = PtrSize * 2 >> >> I think the size offset is `PtrSize + 4`, 4 for `unsigned` capacity. >> How about using Vector<XXX>::m_size instead? > > BTW, VectorBufferOffset is wrongly used in some places of LLInt. It is also used for RefCountedArray (m_argumentValueProfiles). > I think we should use RefCountedArray<XXX>::m_data instead. Ah, whoops. I don't think the LLInt supports using <> in the object offsets syntax, though.

EWS Watchlist

Comment 9 2018-11-09 07:59:13 PST

Comment on attachment 354337 [details] Patch Attachment 354337 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/9923755 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 10 2018-11-09 07:59:14 PST

Created attachment 354343 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 11 2018-11-09 08:03:12 PST

Comment on attachment 354337 [details] Patch Attachment 354337 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9923747 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 12 2018-11-09 08:03:13 PST

Created attachment 354344 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Yusuke Suzuki

Comment 13 2018-11-09 08:05:00 PST

Comment on attachment 354337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354337&action=review >>>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:524 >>>> +const VectorSizeOffset = PtrSize * 2 >>> >>> I think the size offset is `PtrSize + 4`, 4 for `unsigned` capacity. >>> How about using Vector<XXX>::m_size instead? >> >> BTW, VectorBufferOffset is wrongly used in some places of LLInt. It is also used for RefCountedArray (m_argumentValueProfiles). >> I think we should use RefCountedArray<XXX>::m_data instead. > > Ah, whoops. I don't think the LLInt supports using <> in the object offsets syntax, though. How about `const VectorSizeOffset = SimpleJumpTable::branchOffsets::m_size`?

Keith Miller

Comment 14 2018-11-09 08:09:29 PST

Comment on attachment 354337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354337&action=review >>>>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:524 >>>>> +const VectorSizeOffset = PtrSize * 2 >>>> >>>> I think the size offset is `PtrSize + 4`, 4 for `unsigned` capacity. >>>> How about using Vector<XXX>::m_size instead? >>> >>> BTW, VectorBufferOffset is wrongly used in some places of LLInt. It is also used for RefCountedArray (m_argumentValueProfiles). >>> I think we should use RefCountedArray<XXX>::m_data instead. >> >> Ah, whoops. I don't think the LLInt supports using <> in the object offsets syntax, though. > > How about `const VectorSizeOffset = SimpleJumpTable::branchOffsets::m_size`? Ohh! Clever! I think I'd need to make a typedef for branchOffsets but I like the idea!

Keith Miller

Comment 15 2018-11-09 08:22:44 PST

Created attachment 354346 [details] Patch

Yusuke Suzuki

Comment 16 2018-11-09 08:25:56 PST

Comment on attachment 354346 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354346&action=review r=me, nice! > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:523 > +const VectorSizeOffset = Vector::m_size Nice!

Keith Miller

Comment 17 2018-11-09 08:40:16 PST

Committed r238031: <https://trac.webkit.org/changeset/238031>

Radar WebKit Bug Importer

Comment 18 2018-11-09 08:41:26 PST

<rdar://problem/45944924>

Note You need to log in before you can comment on or make changes to this bug.