BuildFix: JavaScriptCore/bytecode/StructureSet.h:262:77: warning

• Component: New Bugs
• Status: RESOLVED FIXED

[Tamas Gergely]

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.

[Tamas Gergely]

Created attachment 235514 [details]
patch

[Darin Adler]

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.

[Filip Pizlo]

(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.

[Darin Adler]

Comment on attachment 235514 [details]
patch

OK.

[WebKit Commit Bot]

Comment on attachment 235514 [details]
patch

Clearing flags on attachment: 235514

Committed r171719: <http://trac.webkit.org/changeset/171719>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.