Using Find it assigning to the concern owner. Below are the find it results --
The result is a list of CLs that change the crashed files. 

Author: danakj
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/880b80edba57b16cf77dc2437c67f27c45815065
Time: Wed Dec 07 01:27:01 2016
File surface_aggregator.cc is changed in this cl (and is part of stack frame #2, "cc::SurfaceAggregator::HandleSurfaceQuad"; frame #3, "cc::SurfaceAggregator::CopyQuadsToPass"; frame #4, "cc::SurfaceAggregator::CopyPasses")
Minimum distance from crash line to modified line: 5. (file: surface_aggregator.cc, crashed on: 478, modified: 473). 

Author: liberato
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/304dc19797fefea519996ef33919b61513716af8
Time: Fri Dec 09 20:55:26 2016
File surface_aggregator.cc is changed in this cl (and is part of stack frame #2, "cc::SurfaceAggregator::HandleSurfaceQuad"; frame #3, "cc::SurfaceAggregator::CopyQuadsToPass"; frame #4, "cc::SurfaceAggregator::CopyPasses")
Minimum distance from crash line to modified line: 64. (file: surface_aggregator.cc, crashed on: 486, modified: 550).

@dankaj -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.

So the code in question here is:

bool Transform::IsIdentityOrIntegerTranslation() const {
  if (!IsIdentityOrTranslation())
    return false;

  bool no_fractional_translation =
      static_cast<int>(matrix_.get(0, 3)) == matrix_.get(0, 3) &&
      static_cast<int>(matrix_.get(1, 3)) == matrix_.get(1, 3) &&
      static_cast<int>(matrix_.get(2, 3)) == matrix_.get(2, 3);

  return no_fractional_translation;
}

We're trying to tell if the floats have a non-integer component, but it is out of the range of ints so UBSAN is not happy - and maybe it would not give the right result then, I'm not sure.

Perhaps these should be floorf(f) == f instead.

Or static_cast<long> maybe? Not sure which is better efficiency-wise.

Oh, nvm, static_cast<long> has the same problem as static_cast<int>, just for larger floats.

Yeh, floats can be large :(

Unfortunately, floorf is worse, because for big floats the imprecision problems kick in, and you get false positives.

This passes with static_cast<int> but fails with floorf

  float max_int = std::numeric_limits<int>::max();
  transform.MakeIdentity();
  transform.Translate3d(0, 0, max_int + 1000.5f);
  EXPECT_FALSE(transform.IsIdentityOrIntegerTranslation());

Does it help to static_cast to double before calling floorf?

For that to work it would have to be the case that for any value representable by a float but stored in a double, taking the floor() of it creates a new representation. How to verify that..

Using doubles does not pass for the following

  float max_int = std::numeric_limits<int>::max();
  transform.MakeIdentity();
  transform.Translate3d(0, 0, max_int + 1000.5f);
  EXPECT_FALSE(transform.IsIdentityOrIntegerTranslation());

  float max_float = std::numeric_limits<float>::max();
  transform.MakeIdentity();
  transform.Translate3d(0, 0, max_float - 0.5f);
  EXPECT_FALSE(transform.IsIdentityOrIntegerTranslation());


I did this:

  double zero = matrix_.get(0, 3);
  double one = matrix_.get(1, 3);
  double two = matrix_.get(2, 3);
  bool no_fractional_translation =
      floor(zero) == zero &&
      floor(one) == one &&
      floor(two) == two;

Of course, static_cast works for both of those.. :)

Altho, the question is why does static_cast work. Because when you take a float non-representable by an int, and static_cast it you get some undefined int. When you turn that back into a float, you get something that is not equal of course.

That means that for any float outside of max/min int, this returns false (even if it has no fractional part), which I verified. Saturated cast would do the same thing, except perhaps right on the boundary - as max_int+0.001 would saturated_cast to max_int which may be reprented as a float the same as max_int+0.001 (for small enough epsilon) and thus you get |true| for a fractional float.

Similarly, if you took static_cast<float>(MAX_INT)-0.0001f, that would (for small enough epsilon) not be represented differently than just static_cast<float>(MAX_INT) so one of the two would give the wrong answer since they both will cast to the same int, even tho both are inside the range of integers.

I think what I'm trying to say is that we can't really return a correct answer here always afaict. But it's okay to return |false| incorrectly, just not to return |true| because returning |false| prevents us from taking shortcuts only.

Also, I guess starting from an integer here doesn't really matter, because that's not reality. If you translate by max_int-30 and that ends up represented as slightly fractional, we can't tell if you really intended an integer or not, we lose that. All we know is that it's fractional. So thinking about it being the wrong answer to go from int to float isn't the right way to think about it I guess.

So, what am I saying...
- If the float is not representable by an int, we can return false.
- If it is representable, then we can static_cast it, but float->int->float==float does not mean that the float has no fraction. It means that if you took the integer and made it a float, you'd get that result.
- When we return true, code will as a result static_cast the values to ints and use them. At that point, we can't actually tell if that is right or wrong, or more right or more wrong than using the floats directly.. so this seems probably just fine.

So I think we just want a IsValueInRangeForNumericType() early out.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/602038a90b8b56b4a96a619962f41f03d42b3c46

commit 602038a90b8b56b4a96a619962f41f03d42b3c46
Author: danakj <danakj@chromium.org>
Date: Mon Dec 19 22:06:46 2016

Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG= 675435 

Review-Url: https://codereview.chromium.org/2585263003
Cr-Commit-Position: refs/heads/master@{#439584}

[modify] https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46/ui/gfx/transform.cc
[modify] https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46/ui/gfx/transform.h
[modify] https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46/ui/gfx/transform_unittest.cc

ClusterFuzz has detected this issue as fixed in range 439552:439637.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4689749039382528

Fuzzer: ifratric-browserfuzzer-v3
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Float-cast-overflow
Crash Address: 
Crash State:
  gfx::Transform::IsIdentityOrIntegerTranslation
  gfx::Transform::IsIdentityOrIntegerTranslation
  cc::MathUtil::ProjectEnclosingClippedRect
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=435261:438085
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=439552:439637

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95oiEF8JPi5seww9Szz2hfq-3T1iMCTBbJpcVSev8SWA4XZW_KdDFl6icXHX79tzX4YgHjOOfdHkBpr5p8wL1x4hzrHYD_DYwB5ippd1_xC7Iv2yuGpOnsVIUqK9bM8sCM59YgBQDxsHhm88jEOr0If5LO1OS5ozyzIUASACzDJIzbmKtI?testcase_id=4689749039382528


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Yiaa

Pada tanggal 20 Des 2016 4.13 PM, "clusterf… via monorail" <
monorail+v2.2757933724@chromium.org> menulis: