If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Investigate Clang-reported memory bugs in gfx

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: gps, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
The Clang Static Analyzer has analyzers for Core Foundation semantics. I've put all of mozilla-central through the static analyzer and it reported a number of potential memory leaks and issues related to graphics.

I'm not an expert on the OS X APIs and I haven't really looked at the reports in detail. There are likely some false positives. I just thought I'd assemble them all in one location where they can be tracked effectively. 

[cairo]
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-G0Zqjh.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-rYK7pn.html#EndPath

[gl]
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-150WJK.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-n4QH35.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-nxhkJO.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-xxtcHl.html#EndPath

[skia]
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-ksUqs8.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-LefYEs.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-XZBxiS.html#EndPath

[thebes]
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-9bJJZW.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-LNeOzf.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-0VgVPA.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-GiQGwa.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-74UjgP.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-7ompG0.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-dhueRI.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-eKkcGp.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-WQpx10.html#EndPath

[widget]
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-33hDHk.html#EndPath
http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-y8mfCR.html#EndPath

http://people.mozilla.org/~gszorc/clang/2011-12-15-13/ contains details of all static analysis results.
Depends on: 711352
Depends on: 711355
Depends on: 711357
(In reply to Gregory Szorc [:gps] from comment #0)

That's an awesome report, thanks! I looked through the thebes errors, and AFAICS there's only one false positive:

> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-9bJJZW.html#EndPath

Here, the analyser can't tell that we're storing the dataRef in the (void*) userData of a harfbuzz blob, to be released later in the DestroyBlobFunc callback. So I believe this is not actually a leak.

> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-LNeOzf.html#EndPath

Filed as bug 711357.

> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-0VgVPA.html#EndPath
> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-GiQGwa.html#EndPath

Filed these two as bug 711352.

> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-74UjgP.html#EndPath
> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-7ompG0.html#EndPath
> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-dhueRI.html#EndPath
> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-eKkcGp.html#EndPath
> http://people.mozilla.org/~gszorc/clang/2011-12-15-13/report-WQpx10.html#EndPath

Filed these five as bug 711355.
Blocks: 712350
The dependencies are fixed. Is this bug?
Flags: needinfo?(jfkthame)
Overall, I don't know. According to comment 1, I looked through the thebes reports (and filed and fixed bugs on the errors it found), but there's no indication that anyone looked into the issues reported elsewhere in gfx.

On the other hand, that was 4 1/2 years ago, so there's a good chance things have changed in the meantime anyway. Perhaps the most useful thing would be to re-run the analysis on current code, and update this bug with a fresh report (or close it, if the problems are gone).
Flags: needinfo?(jfkthame) → needinfo?(gps)
(Reporter)

Comment 4

a year ago
I haven't run the Clang static analysis build in ages. But IIRC sylvestre runs these periodically and may have data to share!
Flags: needinfo?(gps) → needinfo?(sledru)
Yes, I am running scan-build every day: https://people.mozilla.org/~sledru/reports/fx-scan-build/

I think we can close it and use bug 712350 for followup changes.
Status: NEW → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(sledru)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.