I got a compile warning: Source/JavaScriptCore/bytecode/StructureSet.h:262:77: warning: '<anonymous>.JSC::StructureSet::m_pointer' is used uninitialized in this function [-Wuninitialized] This is because the set() method (that is used to initialize the StructureSet object from the constructors directly or through setEmpty() or copyFrom() methods) preserves the reservedFlag from the old value which is not defined when the object is constructed.
Created attachment 235514 [details] patch
Comment on attachment 235514 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=235514&action=review I think the new version of StructureSet was rolled out of the tree; this patch isn’t relevant until we bring it back in. Best would be to make sure Filip fixes this before he re-lands the new code. > Source/JavaScriptCore/bytecode/StructureSet.h:43 > + : m_pointer(0) Please use nullptr. > Source/JavaScriptCore/bytecode/StructureSet.h:49 > + : m_pointer(0) Please use nullptr. > Source/JavaScriptCore/bytecode/StructureSet.h:55 > + : m_pointer(0) Not sure it’s needed in this case. I didn’t carefully study copyFrom, but I don’t think it uses the old m_pointer value.
(In reply to comment #2) > (From update of attachment 235514 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235514&action=review > > I think the new version of StructureSet was rolled out of the tree; this patch isn’t relevant until we bring it back in. It's rolled back in. There are more changes now so this does need to be rebased. But r=me too. This change is fine. > > Best would be to make sure Filip fixes this before he re-lands the new code. > > > Source/JavaScriptCore/bytecode/StructureSet.h:43 > > + : m_pointer(0) > > Please use nullptr. It's a uintptr, so that would be weird. Would it even work? I think it would be weird even if it did work since the point of explicitly setting it is to clear the low tag bits. > > > Source/JavaScriptCore/bytecode/StructureSet.h:49 > > + : m_pointer(0) > > Please use nullptr. Ditto. > > > Source/JavaScriptCore/bytecode/StructureSet.h:55 > > + : m_pointer(0) > > Not sure it’s needed in this case. I didn’t carefully study copyFrom, but I don’t think it uses the old m_pointer value. I think that copyFrom leaves the low bits alone, so we need this change here also.
Comment on attachment 235514 [details] patch OK.
Comment on attachment 235514 [details] patch Clearing flags on attachment: 235514 Committed r171719: <http://trac.webkit.org/changeset/171719>
All reviewed patches have been landed. Closing bug.