360247 - Avoid using floating-point arithmetic during double->string conversion

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

[ This is a spin-off of bug 358569. ]

As the bug 358569 showed, various quirks in floating-point support may seriously affect the results of double->string conversions. Since the code in jsdtoa.c has the ultimate knowledge about the format of double numbers, it would be nice to use that for implementation of double->string conversion that does not use floating-point arithmetic at all.

In this way at least the problems with FPU would not affect the number printouts so one can rely on them, for example, to show FPU problems with arithmetic etc.

Comment 1

[timeless]

afaik, this is inaccurate - not only are strings corrupted, but also basic math operations. Those operations are required to use double precision (not single nor extended). at this point, a plugged gecko violates ECMA specification. Do note that this behavior is both known and not shocking. historically, we forced porters to establish protection for this mode (see OS/2). IMO this bug MUST (RFC: ECMA) be fixed before the next spidermonkey or gecko release.

Comment 2

[Brendan Eich [:brendan]]

See bug 358569 comment 63 second paragraph.

/be

Comment 3

[Keith Victor]

I agree with Timeless.

The solution to the problem is NOT to make sure that the entire codebase can operate flawlessly when a Plugin inadvertently reduces double precision.
The Solution is to make sure Plugins don't do this!

We need to put this code somewhere:

double d12 = 1000000000000.0;
double d1 = 1.0;
if( fabs( d21 * d1 - d12 ) > 0.5 ) ) {
   TellTheUser( "Hey, This Browser has become unstable due to a bad Plugin! Shutdown before someone gets hurt!..." );
}

That would have saved me about a month of work debugging our plugin that inadvertently reduced precision.

Furthermore, I dont think it is possible to do this:

>> it would be nice to use that for implementation of double->string conversion that
>> does not use floating-point arithmetic at all.

Indeed it would be nice.  But, if the double no longer contains enough data, there is no way to get all that data out of the double.
It's like getting two gallons of water out of a one gallon bucket.  You can't code around that.

Comment 4

[Brendan Eich [:brendan]]

Some #ifdef DEBUG code to detect wrong FPU precision mode would be a fine thing. Igor, how about we morph this bug into a request for that protection?

/be

Comment 5

[Keith Victor]

But, Plugin developers don't typically use Debug mozilla code.

Users need to be notified that there is a problem, before something bad happens, IMO.

Comment 6

[Brendan Eich [:brendan]]

Users won't have the first clue what's wrong, or what to do about it.  Plugin developers are going to have to check somehow, without putting cybercrud diags in users' faces.

/be

Comment 7

[Igor Bukanov]

(In reply to comment #3)
> Indeed it would be nice.  But, if the double no longer contains enough data,
> there is no way to get all that data out of the double.

The idea is to convert the bits that inside the double accurately without using FPU so if FPU is broken, at least it would be possible to see exactly how it is broken.

Comment 8

[Igor Bukanov]

(In reply to comment #4)
> Some #ifdef DEBUG code to detect wrong FPU precision mode would be a fine
> thing. Igor, how about we morph this bug into a request for that protection?

I will file another one: I still believe in the long term it would be nice to be able to convert floating point to decimal without using floating point. 

Comment 9

[Igor Bukanov]

(In reply to comment #8)
> (In reply to comment #4)
> > Some #ifdef DEBUG code to detect wrong FPU precision mode would be a fine
> > thing. Igor, how about we morph this bug into a request for that protection?
> 
> I will file another one: I still believe in the long term it would be nice to
> be able to convert floating point to decimal without using floating point. 

See bug 360282.

Comment 10

[Brendan Eich [:brendan]]

Ok, but as noted, this means forking further from dtoa.c.  Who will own this?  If it remains assigned to [e-mail], it's not going to get fixed.

/be

Comment 11

[Igor Bukanov]

(In reply to comment #10)
> Ok, but as noted, this means forking further from dtoa.c.  Who will own this? 
> If it remains assigned to [e-mail], it's not going to get fixed.

I reassign it myself at least to learn exactly what the code is doing.

Comment 12

[Keith Victor]

The user should never see the message, because the Plugin Developer will have found and fixed the problem ( via the annoying popup ).  
No plugin developer will release a plugin that causes such a popup.

I guess it all depends on how important it is to prevent users from running the Browser when the FPU has been reduced.

Comment 13

[Brendan Eich [:brendan]]

(In reply to comment #12)
> The user should never see the message, because the Plugin Developer will have
> found and fixed the problem ( via the annoying popup ).  

Hahahahahhhhhaaaaaa!

Sorry, good one.

Tons of buggy plugins get released.  Often bugs bite only some strange case the plugin dev never hits, because it's far away in the feature-vector hyperspace and only some strange user in the field will tickle it.

> No plugin developer will release a plugin that causes such a popup.

Famous last words.

> I guess it all depends on how important it is to prevent users from running
> the Browser when the FPU has been reduced.

Yes, this is a consideration.  If plugins have such bugs and they bite all the time, that will limit the popularity of such plugins.

I agree we should do something for the plugin developer, but no end user of Firefox should ever see such a dialog.

/be

Comment 14

[timeless]

of note, on windows we do ship an annoying plugin dialog, it's the "this plugin has performed an illegal operation" dialog. 

I'd argue that changing the floating point precision is an illegal operation.

the plugin host could make a note of which plugins it's juggling and test the precision when things return from plugin calls, if it recognizes a plugin has violated this, it should disable the plugin (ok, we don't support this, but we really need to) and toss up a dialog.

if the plugin host at entry (not exit) recognizes that the precision is wrong it should write out a log listing which plugins have been active, and the next time it should try to pinpoint which plugin was the one misbehaving.

ideally the plugin host should be able to conclude which plugin performed the illegal operation.

in the interim, it should send a message to the error console indicating that an illegal operation (change in floating point precision) has occurred along with a list of the plugins for which it is responsible that it suspects might be culpable.

Comment 15

[Igor Bukanov]

I am not working on this.

Comment 16

[Vincent Lefevre]

(In reply to comment #3)
> We need to put this code somewhere:
> 
> double d12 = 1000000000000.0;
> double d1 = 1.0;
> if( fabs( d21 * d1 - d12 ) > 0.5 ) ) {
>    TellTheUser( "Hey, This Browser has become unstable due to a bad Plugin!
> Shutdown before someone gets hurt!..." );
> }

You also need to make sure that extended precision is not used. This may not be important for double->string conversion (but I haven't looked at the code). However with basic arithmetic operations, results can be wrong. Related bug: bug 264912.