1353246 - Undefined behavior in TruncateFloatToInt()

Status: RESOLVED FIXED

(Core :: Web Audio, enhancement, P3)

Description

[Karl Tomlinson (:karlt)]

float(numeric_limits<int>::max()) is rounding up and so this inequality is
false even when f is (float)0x80000000 (without rounding) and cannot be
represented by an int:

http://searchfox.org/mozilla-central/source/dom/media/webaudio/WebAudioUtils.h#193

"A prvalue of a floating point type can be converted to a prvalue of an
 integer type. The conversion truncates; that is, the fractional part is
 discarded. The behavior is undefined if the truncated value cannot be
 represented in the destination type."

(gdb) p (int)(float)2147483647
$2 = -2147483648
(gdb) p /x (int)(float)0x7fffffff
$3 = 0x80000000

Although the -0x80000000 that is returned in this situation on this platform
is negative, the implementation-defined behavior of converting to unsigned for
speex_resampler_init() is to keep the 2s complement representation to give an
unsigned value of 0x80000000, and so no harm follows.

A compiler is permitted by the specification to establish a new world
government instead of returning -0x80000000 from TruncateFloatToInt().

Comment 1

[Karl Tomlinson (:karlt)]

I'm not planning to write a Web Audio test for this because other details of the implementation and compiler behaviours mean that I can't make such a test fail.

Comment 2

[Karl Tomlinson (:karlt)]

The crashtest in https://bugzilla.mozilla.org/attachment.cgi?id=8854332 will exercise this code path.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: bug 1353246 adjust int/float comparison to treat floats just outside the range of int as outside

float(numeric_limits<int>::max()) can round up and so this inequality was
false even when f was (float)0x80000000 (without rounding) and could not be
represented by an int.

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

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 8854341 [details]
bug 1353246 adjust int/float comparison to treat floats just outside the range of int as outside

https://reviewboard.mozilla.org/r/126290/#review129018

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/97001563a3b5
adjust int/float comparison to treat floats just outside the range of int as outside r=padenot

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97001563a3b5