Closed Bug 1033113 Opened 7 years ago Closed 7 years ago

Assertion failure: hasLatin1Chars(), at vm/String.h

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: gkw, Assigned: jandem)

References

Details

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

Attachments

(2 files)

Attached file stack
The upcoming testcase asserts js debug shell on m-c changeset b6408c32a170 with --latin1-strings at Assertion failure: hasLatin1Chars(), at vm/String.h

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140628012028" and the hash "f04ad92a05ea".
The "bad" changeset has the timestamp "20140628012928" and the hash "5c88c5b4fe07".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f04ad92a05ea&tochange=5c88c5b4fe07


Guessing related to bug 1028867 as well, so setting needinfo? from Jan.
Flags: needinfo?(jdemooij)
Nice catch Gary! This is a problem with ropes, here's a minimal testcase:

  var s = "aaaaaaaaaaaaaaaaaaaaaaaaa";
  var latin1Rope1 = "foo" + s;
  var latin1Rope2 = "bar" + latin1Rope1;
  var twoByteRope = "\u1200--" + latin1Rope1;
  twoByteRope.indexOf("111"); // Flatten twoByteRope
  latin1Rope2.indexOf("111"); // Flatten latin1Rope2

When we flatten twoByteRope, we turn latin1Rope1 into a TwoByte dependent string. But latin1Rope1 is also part of latin1Rope2's tree, so we now have a Latin1 rope with a TwoByte child...

I think turning a Latin1 rope into a TwoByte dependent string is the right thing to do; at least I don't see a nice alternative. That means Latin1 rope flattening has to handle TwoByte child nodes, which is a bit unfortunate but not too bad: although it's a TwoByte string, we know the chars must be in the Latin1 range.
Attached patch PatchSplinter Review
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8449239 - Flags: review?(luke)
Flags: needinfo?(jdemooij)
Comment on attachment 8449239 [details] [diff] [review]
Patch

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

::: js/src/vm/String.cpp
@@ +298,5 @@
> +         * ropes) into TwoByte dependent strings. If one of these strings is
> +         * also part of another Latin1 rope tree, we can have a Latin1 rope with
> +         * a TwoByte descendent and we end up here when we flatten it. Although
> +         * the chars are stored as TwoByte, we know they must be in the Latin1
> +         * range, so we can safely deflate here.

wowzers.  Good comment.
Attachment #8449239 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/055d2b0cec57
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.