385259 - nsSelection.cpp relies on undefined behavior

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

We have:
  PRInt32 pad = clipRect.width >> 2;

  if (pad <= 0)

But the second test relies on sign extension happening for >>, which is not specified in the C standard.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

This preserves the desired behavior.

Comment 2

[Mats Palmgren (inactive)]

But clipRect.width is surely non-negative at that point.
So the second test should just be pad==0, no?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: With Mats' suggestion

I'd rather still make the change to / 4 for clarity. The compiler will happily do the strength reduction for us in this case.

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 269304 [details] [diff] [review]
With Mats' suggestion

Oh, either way is fine with me, I wasn't really objecting to
the first patch.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.