Bug 211267 - REGRESSION (r154253): JSC::PropertySlot::m_attributes is uninitialized in constructor
Summary: REGRESSION (r154253): JSC::PropertySlot::m_attributes is uninitialized in con...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on: 119972
Blocks: 212095
  Show dependency treegraph
 
Reported: 2020-04-30 17:38 PDT by David Kilzer (:ddkilzer)
Modified: 2020-05-19 11:30 PDT (History)
8 users (show)

See Also:


Attachments
Patch v1 (2.72 KB, patch)
2020-04-30 17:39 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2020-04-30 17:38:23 PDT
JSC::PropertySlot::m_attributes is uninitialized in constructor.

Found by clang static analyzer with optin.cplusplus.UninitializedObject checker enabled.
Comment 1 David Kilzer (:ddkilzer) 2020-04-30 17:39:04 PDT
Created attachment 398128 [details]
Patch v1
Comment 2 Radar WebKit Bug Importer 2020-04-30 17:40:15 PDT
<rdar://problem/62687958>
Comment 3 Mark Lam 2020-04-30 17:42:20 PDT
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.
Comment 4 David Kilzer (:ddkilzer) 2020-04-30 17:51:22 PDT
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.
Comment 5 David Kilzer (:ddkilzer) 2020-04-30 17:53:53 PDT
Regressed in:

    Bug 119972: Add attributes field to PropertySlot
    <https://bugs.webkit.org/show_bug.cgi?id=119972>
    <https://trac.webkit.org/r154253>
Comment 6 EWS 2020-05-01 03:26:15 PDT
Committed r260993: <https://trac.webkit.org/changeset/260993>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398128 [details].
Comment 7 Darin Adler 2020-05-05 18:16:46 PDT
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."
Comment 8 David Kilzer (:ddkilzer) 2020-05-19 11:30:30 PDT
(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