Closed Bug 1275098 Opened 7 years ago Closed 7 years ago

AudioConverter.cpp is going to have Werror permafail when Gecko 49 merges to Beta on clang-based builds


(Core :: Audio/Video: cubeb, defect, P1)




Tracking Status
firefox47 --- unaffected
firefox48 --- unaffected
firefox49 + fixed


(Reporter: RyanVM, Assigned: jya)



(1 file)

[Tracking Requested - why for this release]: Build bustage when Gecko 49 merges to Beta.

Only appears to affect clang-based builds, FWIW. Looks like bug 1264199 is the only thing to touch this code recently.

10:28:55     INFO -  In file included from /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/dom/media/Unified_cpp_dom_media0.cpp:47:
10:28:55     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/dom/media/AudioConverter.cpp:260:14: error: variable 'error' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
10:28:55     INFO -    } else if (mOut.Format() == AudioConfig::FORMAT_S16) {
10:28:55     INFO -               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10:28:55     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/dom/media/AudioConverter.cpp:270:7: note: uninitialized use occurs here
10:28:55     INFO -    if (error != RESAMPLER_ERR_SUCCESS) {
10:28:55     INFO -        ^~~~~
10:28:55     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/dom/media/AudioConverter.cpp:260:10: note: remove the 'if' if its condition is always true
10:28:55     INFO -    } else if (mOut.Format() == AudioConfig::FORMAT_S16) {
10:28:55     INFO -           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10:28:55     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/dom/media/AudioConverter.cpp:253:12: note: initialize the variable 'error' to silence this warning
10:28:55     INFO -    int error;
10:28:55     INFO -             ^
10:28:55     INFO -              = 0
10:28:55     INFO -  1 error generated.
Flags: needinfo?(jyavenard)
Jean-Yves -- If you're not the right owner for this or if you think you can't get to this right away, please needinfo me to find another owner.  I want to get this resolved quickly.  Thanks!
Rank: 10
Priority: -- → P1
this is a false positive though..

mOut.Format() can *only* be AudioConfig::FORMAT_FLT on desktop or AudioConfig::FORMAT_S16 on Android...

There can be no other way...
To the point where the only case where error would be used "uninitialised" on the line above we have

"MOZ_DIAGNOSTIC_ASSERT(false, "Unsupported data type");"

it *will* crash...

What about just not making invalid warning as fatal error in the first place ? grmmmbbbll
Flags: needinfo?(jyavenard)
Comment on attachment 8755652 [details]
MozReview Request: Bug 1275098: Silence incorrect dumb compiler warning. r?cpearce
Attachment #8755652 - Flags: review?(cpearce) → review+
Assignee: nobody → jyavenard
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Tracking for 49 for potential build bustage.
You need to log in before you can comment on or make changes to this bug.