Assertion failure: max > 0, at js/src/jsapi.h:5914 with saveStacks

RESOLVED FIXED in Firefox 50

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: decoder, Assigned: fitzgen)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla50
x86_64
Linux
assertion, testcase
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
The following testcase crashes on mozilla-central revision e0bc88708ffe (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --no-threads):

saveStack(0.2);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000c540f8 in JS::MaxFrames::MaxFrames (max=0, this=<optimized out>) at js/src/jsapi.h:5914
#0  0x0000000000c540f8 in JS::MaxFrames::MaxFrames (max=0, this=<optimized out>) at js/src/jsapi.h:5914
#1  SaveStack (cx=cx@entry=0x7ffff6965000, argc=1, vp=0x7ffff500b090) at js/src/builtin/TestingFunctions.cpp:1118
#2  0x0000000000ab2b44 in js::CallJSNative (cx=cx@entry=0x7ffff6965000, native=0xc53d60 <SaveStack(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:232
[...]
#15 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7519
rax	0x0	0
rbx	0x7ffff6965000	140737330434048
rcx	0x7ffff6c28a10	140737333332496
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffcae0	140737488341728
rsp	0x7fffffffca50	140737488341584
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fdc740	140737353992000
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff500b090	140737303851152
r13	0x7fffffffca60	140737488341600
r14	0x1	1
r15	0x0	0
rip	0xc540f8 <SaveStack(JSContext*, unsigned int, JS::Value*)+920>
=> 0xc540f8 <SaveStack(JSContext*, unsigned int, JS::Value*)+920>:	movl   $0x0,0x0
   0xc54103 <SaveStack(JSContext*, unsigned int, JS::Value*)+931>:	ud2

Updated

a year ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

a year ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/0916f44729ff
user:        Nick Fitzgerald
date:        Thu Jul 21 23:40:59 2016 -0400
summary:     Bug 1280818 part 1 - Add the ability to capture the stack until the first non-self-hosted frame with the given principals; r=bz,jimb

This iteration took 0.387 seconds to run.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Created attachment 8774417 [details] [diff] [review]
Convert from double to unsigned before checking that the max frames is greater than zero

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=393a80705aea
Attachment #8774417 - Flags: review?(jimb)

Comment 3

a year ago
Comment on attachment 8774417 [details] [diff] [review]
Convert from double to unsigned before checking that the max frames is greater than zero

Review of attachment 8774417 [details] [diff] [review]:
-----------------------------------------------------------------

It's an improvement, but there are still some problems.

::: js/src/builtin/TestingFunctions.cpp
@@ +1110,2 @@
>              return false;
> +        if (maxDouble < 0) {

You actually want to phrase this as `if (! (maxDouble < 0))`, so that if it's NaN, the comparison returns false.

@@ +1113,5 @@
>                                    JSDVG_SEARCH_STACK, args[0], nullptr,
>                                    "not a valid maximum frame count", NULL);
>              return false;
>          }
> +        unsigned max = unsigned(maxDouble);

This is still undefined behavior if maxDouble is beyond the range of unsigned.
Attachment #8774417 - Flags: review?(jimb) → review+
Created attachment 8774519 [details] [diff] [review]
Convert from double to unsigned before checking that the max frames is greater than zero

Now with 100% less undefined behavior!
Attachment #8774519 - Flags: review?(jimb)

Comment 5

a year ago
Pushed by nfitzgerald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/072a565bf24e
Convert from double to unsigned before checking that the max frames is greater than zero; r=jimb
Comment on attachment 8774519 [details] [diff] [review]
Convert from double to unsigned before checking that the max frames is greater than zero

Was supposed to carry the r+. Sorry!
Attachment #8774519 - Flags: review?(jimb) → review+
Attachment #8774417 - Attachment is obsolete: true

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/072a565bf24e
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.