Closed Bug 1350097 Opened 7 years ago Closed 7 years ago

Assertion failure: len <= sizeof(fout), at mozglue/misc/Printf.cpp:280

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: gkw, Assigned: tromey)

References

Details

(Keywords: assertion, bugmon, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

Detailed Crash Information
11.15 KB, text/plain
Details
Bug 1350097 - handle very long floating point output in cvt_f;
59 bytes, text/x-review-board-request
froydnj
: review+
Details 
The following testcase crashes on mozilla-central revision 200182ef1156 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// Adapted from randomly chosen test:
// js/src/tests/test262/language/expressions/exponentiation/applying-the-exp-operator_A11.js
dis(function () {
    x = 1.7976931348623157e308;
})

Backtrace:

#0  0x00000000004739dd in mozilla::PrintfTarget::cvt_f (this=this@entry=0x7fff57b0fe40, d=1.7976931348623157e+308, fmt0=<optimized out>, fmt1=fmt1@entry=0x10ef6b3 "") at mozglue/misc/Printf.cpp:280
#1  0x000000000047494d in mozilla::PrintfTarget::vprint (this=this@entry=0x7fff57b0fe40, fmt=<optimized out>, fmt@entry=0x10ef6b0 "%lf", ap=ap@entry=0x7fff57b0fe90) at mozglue/misc/Printf.cpp:784
#2  0x0000000000b7a5c2 in js::GenericPrinter::vprintf (this=0x7fff57b10100, fmt=0x10ef6b0 "%lf", ap=0x7fff57b0fe90) at js/src/vm/Printer.cpp:85
#3  0x0000000000b7a6b6 in js::GenericPrinter::printf (this=this@entry=0x7fff57b10100, fmt=fmt@entry=0x10ef6b0 "%lf") at js/src/vm/Printer.cpp:72
#4  0x00000000009f24a2 in (anonymous namespace)::ExpressionDecompiler::decompilePC (this=0x7fff57b10080, pc=0x7f3dabb45fbd "<", defIndex=<optimized out>) at js/src/jsopcode.cpp:1980
/snip

For detailed crash information, see attachment.
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/20318a73395e
parent:      4a8979c4c9df
user:        Tom Tromey
date:        Tue Feb 28 10:00:56 2017 -0700
summary:     Bug 1334307 - do not allocate in GenericPrinter::vprintf; r=nbp

Tom, is bug 1334307 a likely regressor?
Flags: needinfo?(ttromey)
Note to self, the test in js/src/tests/test262/language/expressions/exponentiation/applying-the-exp-operator_A11.js contained:

var exponents = [];
exponents[3] = Infinity;
exponents[2] = 1.7976931348623157E308; //largest (by module) finite number
exponents[1] = 1;
exponents[0] = 0.000000000000001;
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2)

> Tom, is bug 1334307 a likely regressor?

Yes, it changed which underlying printf is used for Sprinter::printf, frame #3 in the stack trace.

The code here does this:

    char fout[300];
...
    size_t len = SprintfLiteral(fout, fin, d);
    MOZ_ASSERT(len <= sizeof(fout));

... where |fin| holds the float format string, in this case "%lf".
So this means we're seeing a floating point value that takes more than 300 chars to represent,
or maybe a bug in the platform sprintf.
Flags: needinfo?(ttromey)
I was able to reproduce the bug here.
Assignee: nobody → ttromey
glibc tries to print the value without exponential notation:

(gdb) p fout
$2 = "17976931348623157081452742373170435679807056752584499659891747680315726078002853876058955863276687817154045895351438246423432132688946418276846754670353751698604991057655128207624549009038932894407586"...
According to snprintf the value takes 316 characters to represent.
It's tempting to just bump up the buffer size a bit.
Comment on attachment 8850956 [details]
Bug 1350097 - handle very long floating point output in cvt_f;

https://reviewboard.mozilla.org/r/123468/#review125904

LOL at glibc printing this value out exactly.  Let's not overly complicate this code for the sake of an uncommon case; a little bit more stack is OK.

::: mozglue/misc/Printf.cpp:281
(Diff revision 1)
> -    MOZ_ASSERT(len <= sizeof(fout));
> +    if (MOZ_LIKELY(len <= sizeof(fout)))
> -
> -    return emit(fout, len);
> +        return emit(fout, len);

I think just bumping up the buffer size here to 320 or more, adding a comment explaining why this is sufficient, and turning the assert into MOZ_RELEASE_ASSERT would work just as well.
Attachment #8850956 - Flags: review?(nfroyd)
Sorry, forgot the release assert.  1 sec...
Comment on attachment 8850956 [details]
Bug 1350097 - handle very long floating point output in cvt_f;

https://reviewboard.mozilla.org/r/123468/#review125944
Attachment #8850956 - Flags: review?(nfroyd) → review+
hg error in cmd: hg identify upstream -r tip:
Tom, what's next here? It will be nice to have this landed before the weekend.
Flags: needinfo?(ttromey)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #15)
> Tom, what's next here? It will be nice to have this landed before the
> weekend.

Sorry - I was on PTO and neglected to update my bugzilla info to reflect that.
I see that landing failed.  I've rebased and will retry now.
Flags: needinfo?(ttromey)
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02e8fffc17fb
handle very long floating point output in cvt_f; r=froydnj
https://hg.mozilla.org/mozilla-central/rev/02e8fffc17fb
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
See Also: → 1517433
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: