Bug 216082 – AbstractMacroAssembler::Jump class has uninitialized instance variables

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

[David Kilzer (:ddkilzer)]

AbstractMacroAssembler::Jump class has uninitialized instance variables when using default (and other) constructors.

Found by clang static analyzer using the optin.cplusplus.UninitializedObject checker.

[David Kilzer (:ddkilzer)]

Created attachment 407778 [details]
Patch v1

[David Kilzer (:ddkilzer)]

Comment on attachment 407778 [details]
Patch v1

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

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:685
> +        bool m_is64Bit { false };

Should this be { true } instead since this is only compiled on CPU(ARM64)?  Or do we have 32-bit jumps in arm64[e]?

[Michael Saboff]

Comment on attachment 407778 [details]
Patch v1

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

r=me.  This is good defensive programming, but I don't think that this fixes any latent bugs as these fields are set for the instruction variants that use them.

>> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:685
>> +        bool m_is64Bit { false };
> 
> Should this be { true } instead since this is only compiled on CPU(ARM64)?  Or do we have 32-bit jumps in arm64[e]?

All jumps for ARM64[e] are 64 bit in terms of changes to the PC.  The m_is64Bit is for the compare and branch jumps (CBZ and CBNZ) and relate to the size of the comparison.  The m_is64Bit flag is not used for the other branch types.

[David Kilzer (:ddkilzer)]

Comment on attachment 407778 [details]
Patch v1

Adding cq+ since failures in fast/block/margin-collapse/103.html are happening across multiple patches.  Not related to this patch.

[EWS]

Committed r266530: <https://trac.webkit.org/changeset/266530>

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

[Radar WebKit Bug Importer]

<rdar://problem/68287015>