746577 - ASAN: Opus crash [@nsNativeAudioStream::Write]

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

Happens only in builds with ASAN enabled but is reproducible everytime.


Origin:

case FORMAT_FLOAT32: {
const float* buf = static_cast<const float*>(aBuf);
for (PRUint32 i = 0; i <  samples; ++i) {
  float scaled_value = floorf(0.5 + 32768 * buf[i] * scaled_volume); // <--
  if (buf[i] < 0.0) {
    s_data[i] = (scaled_value < -32768.0) ?
      -32768 :
      short(scaled_value);
  } else {
    s_data[i] = (scaled_value > 32767.0) ?
      32767 :
      short(scaled_value);
  }
}

Comment 1

[Christoph Diehl [:posidron]]

Attachment: sample

Comment 2

[Ralph Giles (:rillian)]

Awesome, that didn't take you long!

Help me out here though, what does "0 bytes to the right of 2592-byte region" mean? It ran off the end of the buf?

Comment 3

[Christian Holler (:decoder)]

(In reply to Ralph Giles (:rillian) from comment #2)
> Help me out here though, what does "0 bytes to the right of 2592-byte
> region" mean? It ran off the end of the buf?

Yes, I assume it's an off-by-one error. Looking at the shadow data, it seems to be accessing one byte outside the valid memory:

0x100026df5754: fb
0x100026df5750: 00 00 00 00 fb fb fb fb

fb means the memory here is not valid.

The error message seems to confirm this, it says the memory region in question is [0x000136fab080,0x000136fabaa0) but 0x000136fabaa0 (which is not included, but the boundary) is accessed.

Comment 4

[Ralph Giles (:rillian)]

For the record, this is testing the patches in Bug 674225.

Comment 5

[Ralph Giles (:rillian)]

I'd guess it's an off-by-many, actually. Looks like I missed a change when I last updated the patch.

Does applying https://github.com/rillian/firefox/commit/ba1bf373e3b71c5c71cee576aeb917a66cdce13d resolve the problem?

Comment 6

[Christoph Diehl [:posidron]]

Yes, that did it.

Comment 7

[Ralph Giles (:rillian)]

Ok, good. I'd say sorry for wasting your time, but this probably also explains why my try builds didn't work when local testing did!

Comment 8

[Daniel Veditz [:dveditz]]

based on comments 5-7 calling this "fixed" before it ever hit mozilla-central.