1409622 - Test failures on OSX when building with clang 5

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P3)

Description

[Mike Hommey [:glandium]]

They all seem to root to the following assertion:

INFO - [Child 1855, Unnamed thread 11facbda0] ###!!! ASSERTION: Bad seconds: '0 <= aSeconds && aSeconds <= TRACK_TICKS_MAX/TRACK_RATE_MAX', file /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.h, line 469

Logs:
https://treeherder.mozilla.org/logviewer.html#?job_id=137763477&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=137690275&repo=autoland

You can check on try by pushing the patch from bug 1409265 along.

Comment 1

[Andreas Pehrson [:pehrsons]]

This is in MediaStreamGraph, so not playback. Furthermore the assert is in a function only called by WebAudio. Paul?

Comment 2

[Paul Adenot (:padenot)]

Thanks for the heads up.

I've done a try push with the clang-5 patches and some stuff to diagnose here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=994800ac79ef6507f7e24e2640f6c586459d9be8

Comment 3

[Paul Adenot (:padenot)]

Of course I pushed without compiling locally, and it broke.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6f57441ab5ccad67359840309205b89f9324fad

Comment 4

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef97382eb39277131967c53ac54deb4d569172be

Comment 5

[Paul Adenot (:padenot)]

Well for some reason it does not print the stack trace and my stuff, now.

Comment 6

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f19294695a06463e53232c69ebfa4d05a937fd8

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to Paul Adenot (:padenot) from comment #5)
> Well for some reason it does not print the stack trace and my stuff, now.

Don't know what happened to the stack trace, but NaNs would explain the lack of output from the printf.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: bug 1409622 initialize mTime even in Stream AudioTimelineEvents

to avoid uninitialized read in WebAudioUtils::ConvertAudioTimelineEventToTicks()

Review commit: https://reviewboard.mozilla.org/r/196968/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/196968/

Comment 9

[Karl Tomlinson (:karlt)]

The alternative would be test mType before use in
WebAudioUtils::ConvertAudioTimelineEventToTicks() and before logging in
SendEventToEngine().  The latter would be a little inconvenient.

The process of initializing in the constructor was begun just for the sake of
keeping Coverity silent for bug 1232646.
I'm continuing that because I guess it will be simplest.

http://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/dom/media/webaudio/WebAudioUtils.cpp#26
http://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/dom/media/webaudio/AudioParam.cpp#158

Comment 10

[Paul Adenot (:padenot)]

Comment on attachment 8925791 [details]
bug 1409622 initialize mTime even in Stream AudioTimelineEvents

https://reviewboard.mozilla.org/r/196968/#review202206

Comment 11

[Paul Adenot (:padenot)]

Comment on attachment 8925791 [details]
bug 1409622 initialize mTime even in Stream AudioTimelineEvents

https://reviewboard.mozilla.org/r/196968/#review202210

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1924564466fe
initialize mTime even in Stream AudioTimelineEvents r=padenot

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1924564466fe