Float-cast-overflow in blink::CSSFontVariationSettingsInterpolationType::ApplyStandardPropertyValue |
|||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5108243556466688 Fuzzer: inferno_twister Job Type: linux_ubsan_chrome Platform Id: linux Crash Type: Float-cast-overflow Crash Address: Crash State: blink::CSSFontVariationSettingsInterpolationType::ApplyStandardPropertyValue blink::CSSInterpolationType::Apply blink::TransitionInterpolation::Apply Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=551565:563900 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5108243556466688 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Nov 29
Automatically adding ccs based on suspected regression changelists: Disable non-composited animations via feature policy by iclelland@chromium.org - https://chromium.googlesource.com/chromium/src/+/9831ecc703d4316100735d9ed9a86702d3a34652 [Squad] style_ in StyleResolverState is always mutable. by futhark@chromium.org - https://chromium.googlesource.com/chromium/src/+/f1b06666866a207acc3a263a83f787267b56ad05 If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.
,
Nov 29
Xida, is this related to the other two bugs? (And is there a pattern here that would fix any others that are waiting to be discovered by clusterfuzz?)
,
Nov 29
This one is odd. If you look at the test case, it is pointing to css_font_variation_settings_interpolation_type.cc at line 216, where there is already a static_cast in there. I don't know how this happens.
,
Nov 30
../../third_party/blink/renderer/core/animation/css_font_variation_settings_interpolation_type.cc:216:28: runtime error: -7.84606e+45 is outside the range of representable values of type 'float' It is also undefined behavior to cast too large or too small double to float. This is essentially a logic bug; we need to decide whether it is ok to just clip here and explicitly do that if so. There should be functions in base to clip double -> float (and if there aren't we should add some).
,
Nov 30
(We would also need to decide whether to clip to infinity or to std::numeric_limits<float>::max()).
,
Nov 30
Probably want saturated_cast<> here (https://cs.chromium.org/chromium/src/base/numerics/safe_conversions.h?l=192&rcl=d8cb4047cb75aa5d5397d6777e9026c7a2b1f8bd) As for 'how do we stop this in the future', you can't detect existing implicit cast cases apart from Ubsan afaik. There are tools like checked_cast (https://cs.chromium.org/chromium/src/base/numerics/safe_conversions.h?l=104&rcl=d8cb4047cb75aa5d5397d6777e9026c7a2b1f8bd) which can be used to DCHECK if values outside of ranges are found.
,
Nov 30
Thanks Stephen, I will take it.
,
Dec 3
Adding a Category to satisfy triage. Perhaps we should declare Stability-UndefinedBehavior to be as categorized as Stability-Crash.
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/242556d729e87df3bf70c0189ecb3dc51ced73c9 commit 242556d729e87df3bf70c0189ecb3dc51ced73c9 Author: Xida Chen <xidachen@chromium.org> Date: Tue Dec 04 17:56:18 2018 [Code health] Do clampTo instead of static_cast Previously in these places we tried to use static_cast to prevent float-cast-overflow, but that doesn't seem to be enough. We should use the clampTo, which is the logic that is currently used in ConsumeFontVariationTag. A layout test is added to make sure that the output is expected. Bug: 910118 Change-Id: If080e914c6959e4570e6695024d21d6b0481bf3d Reviewed-on: https://chromium-review.googlesource.com/c/1357209 Commit-Queue: Xida Chen <xidachen@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Dominik Röttsches <drott@chromium.org> Cr-Commit-Position: refs/heads/master@{#613594} [modify] https://crrev.com/242556d729e87df3bf70c0189ecb3dc51ced73c9/third_party/blink/renderer/core/animation/css_color_interpolation_type.cc [modify] https://crrev.com/242556d729e87df3bf70c0189ecb3dc51ced73c9/third_party/blink/renderer/core/animation/css_font_variation_settings_interpolation_type.cc [modify] https://crrev.com/242556d729e87df3bf70c0189ecb3dc51ced73c9/third_party/blink/web_tests/animations/interpolation/font-variation-settings-interpolation.html
,
Dec 4
,
Dec 5
ClusterFuzz has detected this issue as fixed in range 613589:613594. Detailed report: https://clusterfuzz.com/testcase?key=5108243556466688 Fuzzer: inferno_twister Job Type: linux_ubsan_chrome Platform Id: linux Crash Type: Float-cast-overflow Crash Address: Crash State: blink::CSSFontVariationSettingsInterpolationType::ApplyStandardPropertyValue blink::CSSInterpolationType::Apply blink::TransitionInterpolation::Apply Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=551565:563900 Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=613589:613594 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5108243556466688 See https://github.com/google/clusterfuzz-tools for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Dec 5
ClusterFuzz testcase 5108243556466688 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by ClusterFuzz
, Nov 29Labels: Test-Predator-Auto-Components