1278242 - [Static Analysis][Uninitialized pointer read] In function SurfaceDescriptorX11

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity detected that |mId| is used without initialization:

>>            SurfaceDescriptorX11 tmp = SurfaceDescriptorX11();
>>            (*(v__)) = tmp;
>>            if ((!(Read((&((v__)->get_SurfaceDescriptorX11())), msg__, iter__)))) {
>>                FatalError("Error deserializing Union type");
>>                return false;
>>            }

Also a good idea to initialize |mId| is to be able to prepare the base code for the integration of 525063

Comment 1

[Andi [:andi]]

Attachment: Bug 1278242 - prevent uninitialized pointer read.

Review commit: https://reviewboard.mozilla.org/r/57908/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57908/

Comment 2

[Jeff Muizelaar [:jrmuizel]]

These are public fields that are initialized by the caller. In bug 525063 it might be good to limit the warnings to private fields to start.

I'm still trying to decide whether this is the right fix to the problem.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Is there an annotation that we can use for fields that are intentionally uninitialized?

Comment 4

[Andi [:andi]]

yes you can use: MOZ_INITIALIZED_OUTSIDE_CONSTRUCTOR. Also i've updated the patch with this change. Once the patch for clang-plugin gets landed evey memebr variables that is declared using MOZ_INITIALIZED_OUTSIDE_CONSTRUCTOR will be automatially ignored from analysis.

Comment 5

[Andi [:andi]]

Attachment: Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

Review commit: https://reviewboard.mozilla.org/r/60984/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60984/

Comment 6

[Andi [:andi]]

Comment on attachment 8765759 [details]
Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60984/diff/1-2/

Comment 7

[Jeff Muizelaar [:jrmuizel]]

https://reviewboard.mozilla.org/r/60984/#review57860

Can we use Drawable MOZ_INIT_OUTSIDE_CTOR mGLXPixmap; instead of the syntax you proposed? That or another variant that has MOZ_INIT_OUTSIDE_CTOR on the same line to make it clear that the MOZ_INIT_OUTSIDE_CTOR is part of the variable declaration?

Comment 8

[Andi [:andi]]

sure we can i did this because we are breaching the 80 chars line limit.

Comment 9

[Andi [:andi]]

Comment on attachment 8765759 [details]
Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60984/diff/2-3/

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8765759 [details]
Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

https://reviewboard.mozilla.org/r/60984/#review57868

Comment 11

[Andi [:andi]]

Comment on attachment 8765759 [details]
Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60984/diff/3-4/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3baee224bdd8
ignore initialization check for members from SurfaceDescriptorX11. r=jrmuizel

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3baee224bdd8

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Do we want to uplift that to 50?

Comment 15

[Andi [:andi]]

This has no runtime impact since it's only for our static analysis builds.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

ok, thanks