New issue
Advanced search Search tips

Issue 860347 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocked on:
issue 865552



Sign in to add a comment

Float-cast-overflow in DoInterpolation

Project Member Reported by ClusterFuzz, Jul 4

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6344750492549120

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Float-cast-overflow
Crash Address: 
Crash State:
  DoInterpolation
  blink::OscillatorHandler::Process
  blink::AudioHandler::ProcessIfNecessary
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=551565:556660

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6344750492549120

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jul 4

Components: Blink>WebAudio
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Jul 4

Cc: rtoy@chromium.org
Labels: Test-Predator-Auto-CC
Automatically adding ccs based on suspected regression changelists:

Implement selectable AudioParam rate by rtoy@chromium.org - https://chromium.googlesource.com/chromium/src/+/59636b605b8e4b382a58a4bd8827a3a06d1df42d

Simplify AudioParam::Create interface by rtoy@chromium.org - https://chromium.googlesource.com/chromium/src/+/23eefd32741dc33f5724a8825c24a02e84d54ed5

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.
Cc: kkaluri@chromium.org
rtoy@ Could you please look into this issue...
Cc: -rtoy@chromium.org
Owner: rtoy@chromium.org
Status: Assigned (was: Untriaged)
The test case is setting a parameter value to a value just over the max size_t value.  Need to think a bit on how to handle that case because such a value is perfectly valid according the the WebAudio spec.
My mistake. The problem is that detune is set to a large value.  Converting this to the corresponding frequency value produces infinity.  And various operations on this cause NaN to occur.

We can fix this by clamping the detune values internally to a small range to make sure the resulting frequency is within Nyquist.  Ideally this should be take care of by limiting the valid range of the detune attribute.  See https://github.com/WebAudio/web-audio-api/issues/1698 for the spec issue.

Blockedon: 865552
The behavior of the detune attribute has been clarified so we can look into doing the proper solution here.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 6

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

commit 8a5a6b8b0e4384d17a7e66192963c2e0c0cc2948
Author: Raymond Toy <rtoy@chromium.org>
Date: Thu Sep 06 21:09:32 2018

Change detune min/max limits

The max and min valures for the OscillatorNode.detune parameter has been
reduced according the latest version of the spec.  Make it so.

Also implement clamping of the total frequency to Nyquist so that we don't
generate infinity or NaN doing the computations.  This is ok because anything
beyond Nyquist would output 0 anyway.  But we're careful to preserve the slope
of automations; test added for this as well.

Update expectations for AudioParam/audioparam-nominal-range.html because of
the new warnings produced by clamping detune.

Bug:  865552 ,  860347 
Test: the-oscillatornode-interface/detune-limiting.html
Change-Id: I35a214b0f3156ad8912718dcb277f1967f70df8f
Reviewed-on: https://chromium-review.googlesource.com/1138638
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589299}
[add] https://crrev.com/8a5a6b8b0e4384d17a7e66192963c2e0c0cc2948/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-oscillatornode-interface/detune-limiting.html
[modify] https://crrev.com/8a5a6b8b0e4384d17a7e66192963c2e0c0cc2948/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range-expected.txt
[modify] https://crrev.com/8a5a6b8b0e4384d17a7e66192963c2e0c0cc2948/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range.html
[modify] https://crrev.com/8a5a6b8b0e4384d17a7e66192963c2e0c0cc2948/third_party/blink/renderer/modules/webaudio/oscillator_node.cc

Project Member

Comment 9 by ClusterFuzz, Sep 7

ClusterFuzz has detected this issue as fixed in range 589298:589299.

Detailed report: https://clusterfuzz.com/testcase?key=6344750492549120

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Float-cast-overflow
Crash Address: 
Crash State:
  DoInterpolation
  blink::OscillatorHandler::Process
  blink::AudioHandler::ProcessIfNecessary
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=551565:556660
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=589298:589299

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6344750492549120

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.
Project Member

Comment 10 by ClusterFuzz, Sep 7

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6344750492549120 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