Bug 93150

Summary:

Array checks should use the structure, not the class info

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

93162

Bug Blocks:

91933

Attachments:

Description	Flags
work in progress	none
more	none
the patch	mhahnenberg: review+, buildbot: commit-queue-
patch for landing, hopefully	none

Filip Pizlo

Reported 2012-08-03 12:18:43 PDT

This implies that the LLInt and baseline JIT must profile which structure was observed, so that the DFG may pick the right structure to check on the fast path.

Attachments
work in progress (26.95 KB, patch) 2012-08-03 12:22 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more (33.86 KB, patch) 2012-08-03 18:07 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (71.52 KB, patch) 2012-08-13 23:57 PDT, Filip Pizlo	mhahnenberg: review+ buildbot: commit-queue-	Details Formatted Diff Diff
patch for landing, hopefully (77.51 KB, patch) 2012-08-14 18:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-08-03 12:22:58 PDT

Created attachment 156431 [details] work in progress

Filip Pizlo

Comment 2 2012-08-03 18:07:32 PDT

Created attachment 156494 [details] more

Filip Pizlo

Comment 3 2012-08-13 23:57:06 PDT

Created attachment 158235 [details] the patch

WebKit Review Bot

Comment 4 2012-08-14 00:01:39 PDT

Attachment 158235 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/bytecode/Instruction.h:49: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:79: The parameter name "operation" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 5 2012-08-14 01:22:36 PDT

Comment on attachment 158235 [details] the patch Attachment 158235 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13501104

Mark Hahnenberg

Comment 6 2012-08-14 14:32:01 PDT

Comment on attachment 158235 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=158235&action=review Looks good overall except for minor fix. Also placate windows and style bot please :-) > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:214 > + addSlowCase(branchPtr(NotEqual, Address(regT1, JSCell::classInfoOffset()), TrustedImmPtr(&JSArray::s_info))); Should this be Structure::classInfoOffset()?

Filip Pizlo

Comment 7 2012-08-14 16:00:05 PDT

(In reply to comment #6) > (From update of attachment 158235 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158235&action=review > > Looks good overall except for minor fix. Also placate windows and style bot please :-) > > > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:214 > > + addSlowCase(branchPtr(NotEqual, Address(regT1, JSCell::classInfoOffset()), TrustedImmPtr(&JSArray::s_info))); > > Should this be Structure::classInfoOffset()? Good catch! Thanks!

Filip Pizlo

Comment 8 2012-08-14 18:57:34 PDT

Created attachment 158475 [details] patch for landing, hopefully

WebKit Review Bot

Comment 9 2012-08-14 19:00:39 PDT

Attachment 158475 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/bytecode/Instruction.h:49: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:79: The parameter name "operation" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 10 2012-08-14 19:49:26 PDT

Landed in http://trac.webkit.org/changeset/125637

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