opus - Jun 2018 - Aborting on NaN in CELT - what are the conditions for crash in transient_analysis

Hello,
in this commit in celt_encoder.c
https://git.xiph.org/?p=opus.git;a=commitdiff;h=652c4559f593d3aad78bd5c85a216eeae7859429

I see the note:
+      /* We should never see NaNs here. If we find any, then
something really bad happened and we better abort
+         before it does any damage later on. If these asserts are
disabled (no hardening), then the table
+         lookup a few lines below (id = ...) is likely to crash dur
to an out-of-bounds read. DO NOT FIX
+         that crash on NaN since it could result in a worse issue later on. */

I think I'm exactly in that situation. Opus codec crashes occasionally for
me with Access violation in the transient_analysis function on the line
where it's computing the id from the floating point.

 id = (int)MAX32(0,MIN32(127,floor(64*norm*(tmp[i]+EPSILON))));

Could you please provide more info about what could be the crash reason
there?

Regards,
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20180629/db780dbb/attachment-0001.html>

I believe it would be essentially impossible to cause an access violation
on that line, without some other thread coming in and scribbling all over
your stack. Note that just a few lines before, _every_ value of tmp[i] was
read and written, and here only a few sparse values that never exceed the
first half are read. If something is pointing to this line, then it's
probably incorrect. Do you have a reproduceable test case, or a crash dump?

On Thu, Aug 30, 2018 at 11:38 AM Alexandr Petak <alexandr.petak at
gmail.com>
wrote:
> Hello,
> in this commit in celt_encoder.c
>
>
https://git.xiph.org/?p=opus.git;a=commitdiff;h=652c4559f593d3aad78bd5c85a216eeae7859429
>
> I see the note:
>
> +      /* We should never see NaNs here. If we find any, then something
really bad happened and we better abort
>
> +         before it does any damage later on. If these asserts are disabled
(no hardening), then the table
>
> +         lookup a few lines below (id = ...) is likely to crash dur to an
out-of-bounds read. DO NOT FIX
>
> +         that crash on NaN since it could result in a worse issue later
on. */
>
> I think I'm exactly in that situation. Opus codec crashes occasionally
for
> me with Access violation in the transient_analysis function on the line
> where it's computing the id from the floating point.
>
>  id = (int)MAX32(0,MIN32(127,floor(64*norm*(tmp[i]+EPSILON))));
>
> Could you please provide more info about what could be the crash reason
> there?
>
> Regards,
> Alex
> _______________________________________________
> opus mailing list
> opus at xiph.org
> http://lists.xiph.org/mailman/listinfo/opus
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20180830/8c8840bf/attachment.html>