288993 - In <js/src/fdlibm/*.c>, 2 variables used uninitialized

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P3)

Description

[Serge Gautherie (:sgautherie)]

Spun off from bug 287540 comment 19:

{{
 ------- Additional Comment #17 From Serge GAUTHERIE  2005-04-04 08:59 PDT 
[reply] -------

"Self-Compiled [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b)
Gecko/20050324] (<-- v1.8b1 !) (W2Ksp4)"

Brendan:

To be fixed
{{
d:\mozbuild\mozilla\js\src\fdlibm\e_acos.c(117) : warning C4723: potential
divide by 0
d:\mozbuild\mozilla\js\src\fdlibm\e_asin.c(125) : warning C4723: potential
divide by 0
d:\mozbuild\mozilla\js\src\fdlibm\e_rem_pio2.c(204) : warning C4700: local
variable 'z' used without having been initialized
d:\mozbuild\mozilla\js\src\fdlibm\k_cos.c(125) : warning C4700: local variable
'qx' used without having been initialized
}}

The 2 |divide by 0| seem wanted !? (Is there a way to silence the compiler ??)
The 2 uninitialized variables seem to be fixable !?

(helpwanted)
}}

{{
 ------- Additional Comment #19 From Brendan Eich  2005-04-04 10:26 PDT  [reply]
-------

The e_cos.c and e_sin.c warnings are bogus -- that code wants to synthesize a NaN.

The uninitialized warnings are more serious.  The e_rem_pio2.c one seems like a
typo, or something bad.  It should be fixed.  The k_cos.c one looks easier to fix.

I would appreciate a new, focused bug being filed on just these fdlibm warnings.

/be
}}

Comment 1

[Nicholas Allen]

These changes were intentional to fix bug 24892.

Comment 2

[Brendan Eich [:brendan]]

nallen: can you prove z is set at this point, independent of logic before this
point that depends on x?

    /* set z = scalbn(|x|,ilogb(x)-23) */
        ux.d = x; uz.d = z;

This is in e_rem_pio2.c.

The k_cos.c one seems simpler: qx is clearly uninitialized when assigned to
u.d., but then u's storage is overwritten via the __HI(u) and __LO(u) stores. 
Why not get rid of this line:

                u.d = qx;

and fix this warning?

/be

/be

Comment 3

[Nicholas Allen]

They're both guaranteed to be uninitialized.  But the garbage values at those
locations are never actually used anywhere in the computation.

This construct is a trick to prevent optimizing compilers from choking on this
non-standard C.  If you fix the warning by getting rid of the line, some
compilers (gcc probably) will generate bad code due to the type punning via union.

Comment 4

[Brendan Eich [:brendan]]

nallen: we have GCC-specific extensions to cope -- see jsnum.h.  We should at
least be commenting such an obscure hackaround, but I would rather we use the
right (macro-ized, to minimize ifdefs) GCC extensions.  Are you up for this, or
should I take it?

/be

Comment 5

[Nicholas Allen]

I certainly support well-tested hacks in place of obscure hacks.  You'll have to
take this- VS2005 is still giving me fits working with Mozilla.

Comment 6

[Brendan Eich [:brendan]]

We seem to be free of fdlibm, finally.

/be