Bug 211267 – REGRESSION (r154253): JSC::PropertySlot::m_attributes is uninitialized in constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[David Kilzer (:ddkilzer)]

JSC::PropertySlot::m_attributes is uninitialized in constructor.

Found by clang static analyzer with optin.cplusplus.UninitializedObject checker enabled.

[David Kilzer (:ddkilzer)]

Created attachment 398128 [details]
Patch v1

[Radar WebKit Bug Importer]

<rdar://problem/62687958>

[Mark Lam]

Comment on attachment 398128 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review

r=me

> Source/JavaScriptCore/runtime/PropertySlot.h:404
> +    } m_additionalData { { 0, 0 } };

Interesting.  I never knew that we can do this.

[David Kilzer (:ddkilzer)]

Comment on attachment 398128 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review

>> Source/JavaScriptCore/runtime/PropertySlot.h:404
>> +    } m_additionalData { { 0, 0 } };
> 
> Interesting.  I never knew that we can do this.

I think it works because both union types have two instance variables.  I guess gcc and MSVC++ will tell us if it's portable, though.

[David Kilzer (:ddkilzer)]

Regressed in:

    Bug 119972: Add attributes field to PropertySlot
    <https://bugs.webkit.org/show_bug.cgi?id=119972>
    <https://trac.webkit.org/r154253>

[EWS]

Committed r260993: <https://trac.webkit.org/changeset/260993>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398128 [details].

[Darin Adler]

Comment on attachment 398128 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review

>>> Source/JavaScriptCore/runtime/PropertySlot.h:404
>>> +    } m_additionalData { { 0, 0 } };
>> 
>> Interesting.  I never knew that we can do this.
> 
> I think it works because both union types have two instance variables.  I guess gcc and MSVC++ will tell us if it's portable, though.

No, it’s only domAttribute that is initialized. I looked it up:

"When a union is initialized by aggregate initialization, only its first non-static data member is initialized."

[David Kilzer (:ddkilzer)]

(In reply to Darin Adler from comment #7)
> Comment on attachment 398128 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398128&action=review
> 
> >>> Source/JavaScriptCore/runtime/PropertySlot.h:404
> >>> +    } m_additionalData { { 0, 0 } };
> >> 
> >> Interesting.  I never knew that we can do this.
> > 
> > I think it works because both union types have two instance variables.  I guess gcc and MSVC++ will tell us if it's portable, though.
> 
> No, it’s only domAttribute that is initialized. I looked it up:
> 
> "When a union is initialized by aggregate initialization, only its first
> non-static data member is initialized."

Bug 212095: Make union initializers for JSC::PropertySlot more explicit about which field is being initialized