Bug 75595

Summary:

Default HashTraits for Opcode don't work for Opcode = 0

Product:

WebKit

Reporter:

Michael Saboff <msaboff>

Component:

JavaScriptCore

Assignee:

Michael Saboff <msaboff>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ggaren, harrison, webkit.review.bot

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	none
Patch with indent change for stylebot	ggaren: review-
Updated patch as discussed	oliver: review+

Michael Saboff

Reported 2012-01-04 18:24:17 PST

Opcode is defined as a void* and the value '0' is used in a HaspMap when COMPUTED_GOTO_INTERPRETER is enabled. An Opcode specific HashTraits needs to be created to account for this.

Attachments
Proposed patch (1.95 KB, patch) 2012-01-04 18:44 PST, Michael Saboff	no flags	Details Formatted Diff Diff
Patch with indent change for stylebot (1.89 KB, patch) 2012-01-04 18:51 PST, Michael Saboff	ggaren: review-	Details Formatted Diff Diff
Updated patch as discussed (2.80 KB, patch) 2012-01-05 18:12 PST, Michael Saboff	oliver: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Michael Saboff

Comment 1 2012-01-04 18:44:04 PST

Created attachment 121202 [details] Proposed patch

WebKit Review Bot

Comment 2 2012-01-04 18:46:25 PST

Attachment 121202 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/interpreter/Interpreter.h:45: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Michael Saboff

Comment 3 2012-01-04 18:51:21 PST

Created attachment 121203 [details] Patch with indent change for stylebot

Geoffrey Garen

Comment 4 2012-01-05 11:21:49 PST

Comment on attachment 121203 [details] Patch with indent change for stylebot A few comments: 1. I believe you can just use WTF::UnsignedWithZeroKeyHashTraits. You can do this by putting UnsignedWithZeroKeyHashTraits in the declaration of any hash table using Opcode as key, or by declaring HashTraits<Opcode> to inherit from UnsignedWithZeroKeyHashTraits. This is better than duplicating UnsignedWithZeroKeyHashTraits. 2. This code should not be conditional on #if ENABLE(COMPUTED_GOTO_INTERPRETER). That feature define does not have any bearing on whether the Opcode type has 0 in its domain. 3. The best place to put HashTraits for Opcode is Opcode.h. That way, it's impossible to put Opcode into a hash table inocrrectly.

Geoffrey Garen

Comment 5 2012-01-05 11:50:23 PST

Even better solution: fix up Interpreter::initialize, as we discussed.

Michael Saboff

Comment 6 2012-01-05 18:12:28 PST

Created attachment 121379 [details] Updated patch as discussed

David Harrison

Comment 7 2012-01-06 10:11:07 PST

rdar://10649745

Geoffrey Garen

Comment 8 2012-01-06 10:53:57 PST

I think you can remove the whole "if (canUseJIT)" case from Interpreter::initialize. m_opcodeTable isn't used at all if Opcodes are not pointers.

Michael Saboff

Comment 9 2012-01-06 14:15:46 PST

Committed r104338: <http://trac.webkit.org/changeset/104338>

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