Closed Bug 1228704 Opened 4 years ago Closed 3 years ago

[Static Analysis] OVERFLOW_BEFORE_WIDEN defect reported in SurfaceCache.cpp

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1251742
Tracking Status
firefox45 --- affected

People

(Reporter: xidorn, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: gfx-noted)

There is a new defect detected by Coverity Scan, which, I think, is worth a fix, by either making the variables int32_t instead if we don't think it would ever overflow, or casting the first factor to int64_t before applying the multiplication.

Details:
> *** CID 1340239:    (OVERFLOW_BEFORE_WIDEN)
> /image/SurfaceCache.cpp: 331 in mozilla::image::ImageSurfaceCache::LookupBestMatch(const mozilla::image::SurfaceKey &)()
> 325           SurfaceKey bestMatchKey = bestMatch->GetSurfaceKey();
> 326
> 327           // Compare sizes. We use an area-based heuristic here instead of computing a
> 328           // truly optimal answer, since it seems very unlikely to make a difference
> 329           // for realistic sizes.
> 330           int64_t idealArea = idealKey.Size().width * idealKey.Size().height;
> >>>     CID 1340239:    (OVERFLOW_BEFORE_WIDEN)
> >>>     Potentially overflowing expression "aSurfaceKey->Size().width * aSurfaceKey->Size().height" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "int64_t" (64 bits, signed).
> 331           int64_t surfaceArea = aSurfaceKey.Size().width * aSurfaceKey.Size().height;
> 332           int64_t bestMatchArea =
> 333             bestMatchKey.Size().width * bestMatchKey.Size().height;
> 334
> 335           // If the best match is smaller than the ideal size, prefer bigger sizes.
> 336           if (bestMatchArea < idealArea) {
> /image/SurfaceCache.cpp: 333 in mozilla::image::ImageSurfaceCache::LookupBestMatch(const mozilla::image::SurfaceKey &)()
> 327           // Compare sizes. We use an area-based heuristic here instead of computing a
> 328           // truly optimal answer, since it seems very unlikely to make a difference
> 329           // for realistic sizes.
> 330           int64_t idealArea = idealKey.Size().width * idealKey.Size().height;
> 331           int64_t surfaceArea = aSurfaceKey.Size().width * aSurfaceKey.Size().height;
> 332           int64_t bestMatchArea =
> >>>     CID 1340239:    (OVERFLOW_BEFORE_WIDEN)
> >>>     Potentially overflowing expression "bestMatchKey.Size().width * bestMatchKey.Size().height" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "int64_t" (64 bits, signed).
> 333             bestMatchKey.Size().width * bestMatchKey.Size().height;
> 334
> 335           // If the best match is smaller than the ideal size, prefer bigger sizes.
> 336           if (bestMatchArea < idealArea) {
> 337             if (surfaceArea > bestMatchArea) {
> 338               bestMatch = surface;
> /image/SurfaceCache.cpp: 330 in mozilla::image::ImageSurfaceCache::LookupBestMatch(const mozilla::image::SurfaceKey &)()
> 324
> 325           SurfaceKey bestMatchKey = bestMatch->GetSurfaceKey();
> 326
> 327           // Compare sizes. We use an area-based heuristic here instead of computing a
> 328           // truly optimal answer, since it seems very unlikely to make a difference
> 329           // for realistic sizes.
> >>>     CID 1340239:    (OVERFLOW_BEFORE_WIDEN)
> >>>     Potentially overflowing expression "idealKey->Size().width * idealKey->Size().height" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "int64_t" (64 bits, signed).
> 330           int64_t idealArea = idealKey.Size().width * idealKey.Size().height;
> 331           int64_t surfaceArea = aSurfaceKey.Size().width * aSurfaceKey.Size().height;
> 332           int64_t bestMatchArea =
> 333             bestMatchKey.Size().width * bestMatchKey.Size().height;
> 334
> 335           // If the best match is smaller than the ideal size, prefer bigger sizes.

This defect is initially introduced in bug 1119774, but the code was moved for refactor recently in bug 1186796, so it is detected again.
Summary: OVERFLOW_BEFORE_WIDEN defect reported in SurfaceCache.cpp → [Static Analysis] OVERFLOW_BEFORE_WIDEN defect reported in SurfaceCache.cpp
What should we do this with :seth?
Flags: needinfo?(seth)
Whiteboard: gfx-noted
(In reply to Benoit Girard (:BenWa) from comment #1)
> What should we do this with :seth?

Let's go ahead and fix this as part of the upcoming SurfaceCache refactoring.
Depends on: 1292392
Flags: needinfo?(seth.bugzilla)
Should be fixed by bug 1251742 and bug 1228314.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1251742
You need to log in before you can comment on or make changes to this bug.