Closed Bug 1278242 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [gfx-noted] CID 1362536, CID 1362537, CID 1362538, CID 1362539, CID 1362540, CID 1362541, CID 750461)

Attachments

(1 file, 1 obsolete file)

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
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.
Is there an annotation that we can use for fields that are intentionally uninitialized?
Flags: needinfo?(bpostelnicu)
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.
Flags: needinfo?(bpostelnicu)
Attachment #8760262 - Attachment is obsolete: true
Attachment #8760262 - Flags: review?(jmuizelaar)
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/
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?
sure we can i did this because we are breaching the 80 chars line limit.
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 on attachment 8765759 [details]
Bug 1278242 - ignore initialization check for members from SurfaceDescriptorX11.

https://reviewboard.mozilla.org/r/60984/#review57868
Attachment #8765759 - Flags: review?(jmuizelaar) → review+
Whiteboard: CID 1362536, CID 1362537, CID 1362538, CID 1362539, CID 1362540, CID 1362541, CID 750461 → [gfx-noted] CID 1362536, CID 1362537, CID 1362538, CID 1362539, CID 1362540, CID 1362541, CID 750461
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3baee224bdd8
ignore initialization check for members from SurfaceDescriptorX11. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/3baee224bdd8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Do we want to uplift that to 50?
This has no runtime impact since it's only for our static analysis builds.
You need to log in before you can comment on or make changes to this bug.