|Summary:||Default HashTraits for Opcode don't work for Opcode = 0|
|Product:||WebKit||Reporter:||Michael Saboff <msaboff>|
|Severity:||Normal||CC:||ggaren, harrison, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Michael Saboff 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.
Comment 2 WebKit Review Bot 2012-01-04 18:46:25 PST
Comment 3 Michael Saboff 2012-01-04 18:51:21 PST
Created attachment 121203 [details] Patch with indent change for stylebot
Comment 4 Geoffrey Garen 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.
Comment 5 Geoffrey Garen 2012-01-05 11:50:23 PST
Even better solution: fix up Interpreter::initialize, as we discussed.
Comment 6 Michael Saboff 2012-01-05 18:12:28 PST
Created attachment 121379 [details] Updated patch as discussed
Comment 8 Geoffrey Garen 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.