Bug 135287 - BuildFix: JavaScriptCore/bytecode/StructureSet.h:262:77: warning
Summary: BuildFix: JavaScriptCore/bytecode/StructureSet.h:262:77: warning
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-25 07:31 PDT by Tamas Gergely
Modified: 2014-07-28 19:12 PDT (History)
3 users (show)

See Also:


Attachments
patch (1.41 KB, patch)
2014-07-25 07:47 PDT, Tamas Gergely
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tamas Gergely 2014-07-25 07:31:49 PDT
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.
Comment 1 Tamas Gergely 2014-07-25 07:47:30 PDT
Created attachment 235514 [details]
patch
Comment 2 Darin Adler 2014-07-27 23:34:07 PDT
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.
Comment 3 Filip Pizlo 2014-07-28 08:03:32 PDT
(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 4 Darin Adler 2014-07-28 18:38:25 PDT
Comment on attachment 235514 [details]
patch

OK.
Comment 5 WebKit Commit Bot 2014-07-28 19:12:04 PDT
Comment on attachment 235514 [details]
patch

Clearing flags on attachment: 235514

Committed r171719: <http://trac.webkit.org/changeset/171719>
Comment 6 WebKit Commit Bot 2014-07-28 19:12:07 PDT
All reviewed patches have been landed.  Closing bug.