If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

TM: "Assertion failure: "Constantly false guard detected": 0"

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: gkw, Assigned: njn)

Tracking

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

Trunk
x86
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 beta8+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
for (a = 0; a < 9; a++) {
  "".charAt(3 / 0)
}

asserts js debug shell on TM changeset a409054e1395 with -j at Assertion failure: "Constantly false guard detected": 0
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
Summary: "Assertion failure: "Constantly false guard detected": 0" → TM: "Assertion failure: "Constantly false guard detected": 0"
(Reporter)

Comment 1

7 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   48671:fd0411f5ce7f
user:        Andreas Gal
date:        Thu Aug 05 22:54:34 2010 -0700
summary:     Optimize string[idx] on trace (584499, r=lw).
Blocks: 584499

Updated

7 years ago
blocking2.0: ? → beta8+

Comment 2

7 years ago
Created attachment 480413 [details] [diff] [review]
Proposed patch

The same assertion occurs with this testcase:
for (a = 0; a < 9; a++) {
  "abc".charCodeAt(1.5);
}

It's a result of getCharAt/getCharCodeAt calling makeNumberInt32 with a double that is not integral.

I've uploaded a patch that seems to fix it. Though my understanding of the this part of the code base is quite limited (to say the least), so it probably isn't the best solution.
(Reporter)

Comment 3

7 years ago
Comment on attachment 480413 [details] [diff] [review]
Proposed patch

Switching to gal for feedback (I hope I'm correct) - also AIUI a test should also be provided..
Attachment #480413 - Flags: feedback?(gal)
(Assignee)

Comment 4

7 years ago
Comment on attachment 480413 [details] [diff] [review]
Proposed patch

Stealing the review, I've been looking at this stuff lately.
Attachment #480413 - Flags: feedback?(gal) → feedback?(nnethercote)
(Assignee)

Comment 5

7 years ago
Changing the makeNumberInt32() calls to demote() doesn't seem right.  I think demote() is only supposed to be used on values for which isPromoteInt() succeeds.  If the test was changed to this:

  for (a = 0; a < 9; a++) {
    "....".charAt(1 / 3)
  }

demote() would be called on an immediate with value 1/3.
(Assignee)

Comment 6

7 years ago
The bounds check looks good, though, because it matches the charCodeAt case.  Just adding the bounds check is enough to make the test in comment 0 pass, but the one in comment 5 asserts.

Comment 7

7 years ago
Created attachment 480872 [details] [diff] [review]
Proposed patch 2
Attachment #480413 - Attachment is obsolete: true
Attachment #480872 - Flags: feedback?(nnethercote)
Attachment #480413 - Flags: feedback?(nnethercote)
(Assignee)

Comment 8

7 years ago
Comment on attachment 480872 [details] [diff] [review]
Proposed patch 2

I'm pretty sure that isn't right.  The 'i' you see at record-time may be different when the code is executed again later.

Bug 601009 comment 3 has a proposal for a more general solution to avoiding this assertion, which will fix this bug.  I'll work on it tomorrow.  Thanks for the patches!
Attachment #480872 - Flags: feedback?(nnethercote) → feedback-
(Assignee)

Comment 9

7 years ago
(In reply to comment #8)
> 
> Bug 601009 comment 3 has a proposal for a more general solution to avoiding
> this assertion, which will fix this bug.  I'll work on it tomorrow.

This patch is now up, it fixes the test case in comment 0.
(Assignee)

Updated

7 years ago
Depends on: 601009
Setting assignee field so this doesn't look like an unassigned blocker.
Assignee: general → nnethercote
(Assignee)

Comment 11

7 years ago
Fixed by bug 601009.
Severity: critical → normal
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.