533397 - Debug builds exit when converting "1e-400" to a Number

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Run the testcase in the url field, or this in shell:

  parseFloat("1e-400")

This causes a debug build to print "Zero passed to d2b" and then exit.  This seems to come from dtoa.c, which has:

1271                         Bug("Zero passed to d2b");

with Bug() defined as:

176 #ifdef DEBUG
177 #include "stdio.h"
178 #define Bug(x) {fprintf(stderr, "%s\n", x); exit(1);}
179 #endif

In an opt build I just get 0, as more or less expected.  The original testcase I ran into this on was actually passing this string:

  '{ "very_big_number":1e+400, "very_small_number":1e-400 }'

to JSON.parse to hit this bug.  Note that jsdtoa and prdtoa have the same code; should I file separate bugs on those?

Comment 1

[Andreas Gal :gal]

I am going to hide this until we had a chance to analyze whats going on behind the scenes. We can reopen if this is harmless.

Comment 2

[Reed Loden [:reed]]

This is the JS equivalent of bug 414772. I don't think it's a security issue. We can just use that same patch.

Comment 3

[Reed Loden [:reed]]

Attachment: patch - v1

Direct port of the patch from bug 414772.

Comment 4

[Andreas Gal :gal]

Looks harmless. Opening up the bug and removing the flags.

Comment 5

[Andreas Gal :gal]

Comment on attachment 416532 [details] [diff] [review]
patch - v1

Stealing. I don't think Blake minds. The code was already reviewed for the other copy of dtoa anyway.

Comment 6

[Reed Loden [:reed]]

Requesting approval to land this on trunk and 1.9.2, but I think this may be fine to land on trunk based on the patch being "not part of the Firefox or Fennec builds", as it only affects debug builds, which aren't the normal type of build.

Comment 7

[Andreas Gal :gal]

Yeah, pretty sure you don't need approval. Just land on trunk and 192. Its NPOTB.

Comment 8

[Wan-Teh Chang]

Comment on attachment 416532 [details] [diff] [review]
patch - v1

r=wtc.

Comment 9

[Boris Zbarsky [:bzbarsky]]

We need to change jsdtoa as well, right?

Comment 10

[Reed Loden [:reed]]

(In reply to comment #9)
> We need to change jsdtoa as well, right?

Nope, problem doesn't exist there.

Comment 11

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/ce80437ab5bb
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c438ba15483a

Comment 12

[Boris Zbarsky [:bzbarsky]]

What about prdtoa.c?  That does seem to have the bug.

Comment 13

[Wan-Teh Chang]

bz: prdtoa.c has been fixed in NSPR bug 414772.

Since NSPR and JS are separate products, every dtoa bug is
covered by an NSPR bug and a JS bug.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Ah, I see. It would have been really nice to file the JS bug when that one was fixed, so I didn't have to re-debug it 3 months later.  :(

I assume that the NSPR version that bug 414772 was fixed for hasn't been imported into mozilla-central yet?

Comment 15

[Wan-Teh Chang]

The NSPR bug fix is already in mozilla-central, mozilla-1.9.2,
and mozilla-1.9.1.  This bug is not worth backporting to old
release branches, but it piggybacked on the fix for a security
vulnerability in dtoa.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Ah, I see.  I was confused by the other "zero passed to d2b" that's still there.  OK.